Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate 8-byte span IDs to conform with OpenTelemetry spec (instead of 16-byte currently) #171

Closed
brentax opened this issue Sep 12, 2022 · 5 comments
Labels
type: enhancement New feature or request

Comments

@brentax
Copy link

brentax commented Sep 12, 2022

Thank you for making this tool! It made it super easy to instrument our CircleCI builds and deploys.

Is your feature request related to a problem? Please describe.
tl;dr of problem: buildevents cmd generates 16-byte span IDs, but OpenTelemetry specifies (and the Python library implements) an 8-byte limit for span IDs. This blocks us from connecting Python/OpenTelemetry spans to a buildevents trace.

Longer version: I am using buildevents to instrument CircleCI workflows that call many Python scripts. I am using OpenTelemetry to instrument some of those Python scripts. I am trying to include the spans generated in the Python scripts as child spans to those generated by buildevents by using the HONEYCOMB_TRACE propagation data sent by buildevents cmd. buildevents is generating 16-byte span IDs, but OpenTelemetry only accepts 8-byte span IDs. I have been unable to figure out a way to set a root span in Python's OpenTelemetry implementation with the span IDs I get from buildevents.

Describe the solution you'd like
Ideally, buildevents would generate 8-byte span IDs to match the OpenTelemetry spec. I noticed the 16-byte IDs happening here in cmd_cmd.go, but it may be happening in other parts of the code.

Describe alternatives you've considered

  • Don't include Python spans as children of the overall CircleCI workflow traces. This is what we'll be doing for now.
  • Patch our own version of OpenTelemetry to allow larger Span IDs. I'd rather not incur the extra overhead of managing our own fork and keeping it updated with upstream.
  • Patch our own version of buildevents to generate smaller Span IDs. Similar issue with the overhead. Plus we don't currently have any go code or build systems, so we'd also need to manage that.
  • I'm open to other alternatives that we haven't considered yet, would love a workaround!

Additional context
I think I covered it all, but happy to answer any questions.

@brentax brentax added the type: enhancement New feature or request label Sep 12, 2022
@JamieDanielson JamieDanielson added the status: oncall Flagged for awareness from Honeycomb Telemetry Oncall label Sep 12, 2022
@JamieDanielson JamieDanielson self-assigned this Sep 12, 2022
@JamieDanielson
Copy link
Contributor

Thank you for providing this detail @brentax!

For implementing, it may be best to add a new boolean flag to enable 8-byte span IDs instead of 16-byte span IDs, to prevent breakage for current users while still allowing the 16-byte option for those who want it. We'll have to think a bit on this, and have added to our backlog.

@brentax
Copy link
Author

brentax commented Sep 16, 2022

Thanks @JamieDanielson!

A boolean flag would work well for our use case. I'll follow this issue for any updates.

@pkanal pkanal removed the status: oncall Flagged for awareness from Honeycomb Telemetry Oncall label Nov 14, 2022
@ham1
Copy link

ham1 commented Mar 10, 2023

Could we perhaps allow specifying the span id for each command, so that this and other use cases can be catered for?

@lizthegrey
Copy link
Member

This is partially addressed with honeycombio/buildevents-orb#110

@lizthegrey
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants