-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[ottl] Improve how to check for nil parent_span_id #32918
Comments
Pinging code owners for processor/transform: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley. See Adding Labels via Comments if you do not have permissions to add labels yourself. |
Pinging code owners for pkg/ottl: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley. See Adding Labels via Comments if you do not have permissions to add labels yourself. |
I don't have the full context necessary to make an informed decision here, but I like the
|
Hello! I would like to work on this issue cc @evan-bradley |
To clarify the requirements:
Please confirm this is the expected behavior @evan-bradley @TylerHelmuth Thank you! |
@odubajDT that sounds right. This function should only be exposed for ottlspan context. |
While having an So I think people are going to write
I am no Go expert by a long shot, but looking at https://github.com/open-telemetry/opentelemetry-collector/blob/30de6856759ea2387c1a946de318429b4f0f503f/pdata/internal/data/spanid.go#L39 it seems like it isn't possible to have trace/span ids that are actually all zeros. And if you change the implementation of the ottl
nil when you pass in 0x0000000000000000 , you won't break any existing parent_span_id == SpanID(0x0000000000000000) either.
|
**Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Introduced `IsRootSpan()` converter function which returns `true` if the span in the corresponding context is root, that means its `parent_span_id` equals to hexadecimal representation of zero. In all other scenarios function returns `false`. **Link to tracking Issue:** #32918 **Testing:** - unit tests - e2e tests **Documentation:** <Describe the documentation added.> - README entry --------- Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com> Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
@evanderkoogh Thank you for letting us know you also ran into this. I've been thinking it over, I'm still a little split on whether to represent an empty span ID as Right now the representation has to be all zeroes because that's how Go represents the "zero value" for |
@evan-bradley, I agree that it is a change that is tricky and should include a bit more thought than someone with no prior knowledge of the codebase going "I am pretty sure".. :)
I can't quickly find out where the compare happens for |
Actually.. looking at the code again, if we added a case for
And then added a case for it in
We could override the behaviour to make |
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
This issue has been closed as inactive because it has been stale for 120 days with no activity. |
Component(s)
No response
Is your feature request related to a problem? Please describe.
Right now if you want to perform a transform only on a root span you have to write a condition that is either
parent_span_id == SpanID(0x0000000000000000)
or
parent_span_id.string == "0000000000000000"
This is because pdata represents empty parent span id as
[0, 0, 0, 0, 0, 0, 0, 0, 0]
Describe the solution you'd like
I want to make it more intuitive to identify a root span. While we have a solution, I wouldn't expect a regular user to understand that
== nil
or== ""
will not work and that they need to represent an all-zero span id instead.I think we could accomplish this with a new
IsRootSpan
converter that takes no parameters and returns returns true if the current context's parent span id is[0, 0, 0, 0, 0, 0, 0, 0, 0]
. But this solution wouldn't help users who are looking for empty span id or trace id.Another option is to update the way that the contexts handle empty span/trace ids. Instead of returning default values, they could return nil. This would be a breaking change and will remove the ability to actually interact with a trace/span id that actually is all zeros.
Describe alternatives you've considered
We could do nothing.
Additional context
This issue spawned from a user needing to modify only root spans, at which point I realized identifying a root span is difficult to stumble upon.
The text was updated successfully, but these errors were encountered: