Skip to content
This repository has been archived by the owner on Dec 20, 2021. It is now read-only.

Transcribe attributes from span links to events #96

Merged
merged 1 commit into from
Dec 28, 2020

Conversation

seh
Copy link
Contributor

@seh seh commented Oct 14, 2020

Links attached to OpenTelemetry spans carry attributes like spans and events. Since we publish links as Honeycomb events, and events all carry fields, transcribe attributes on links as fields on the corresponding Honeycomb events.

Include OpenTelemetry resource attributes as the underlay, and shadow those resource attributes if necessary with the link-level attributes, as we did for spans and span events in #71.

@seh seh requested a review from a team October 14, 2020 17:47
@seh
Copy link
Contributor Author

seh commented Oct 14, 2020

Note that this patch is stacked on top of #95, but GitHub won't allow me to represent that properly here.

@MikeGoldsmith
Copy link
Contributor

@seh I've merged #95 but it looks like this PR has got a little knotted up. Please could you rebase?

Thanks!

@seh seh force-pushed the transcribe-span-link-attributes branch from 0ae0c34 to db66e5d Compare October 16, 2020 12:40
@seh
Copy link
Contributor Author

seh commented Oct 16, 2020

Please could you rebase?

Yes, it looks cleaner now. Please take another look.

@asdvalenzuela
Copy link

@seh Apologies for the hold up with this PR. If you would please rebase again I will make sure this gets reviewed promptly. 🙏

Links attached to OpenTelemetry spans carry attributes like spans and
events. Since we publish links as Honeycomb events, and events all
carry fields, transcribe attributes on links as fields on the
corresponding Honeycomb events. Include OpenTelemetry resource
attributes as the underlay, and shadow those resource attributes if
necessary with the link-level attributes.
@seh seh force-pushed the transcribe-span-link-attributes branch from db66e5d to 9a2e994 Compare December 23, 2020 13:40
@seh
Copy link
Contributor Author

seh commented Dec 23, 2020

If you would please rebase again I will make sure this gets reviewed promptly.

This was a blast from the past. Please see the latest commit.

Copy link

@asdvalenzuela asdvalenzuela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question for clarification, otherwise looks good! Thanks!


// Treat resource-defined attributes as underlays, with any same-keyed span attributes taking
// precedence. Apply them first.
applyResourceAttributes(ev)
if len(e.serviceName) != 0 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove adding service_nameto the event here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See lines 508-511 above. It still happens as part of the call to applyResourceAttributes at line 521.

@asdvalenzuela asdvalenzuela merged commit 318ed70 into honeycombio:main Dec 28, 2020
@seh seh deleted the transcribe-span-link-attributes branch December 29, 2020 00:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants