[ottl] Improve how to check for nil parent_span_id

[TylerHelmuth]

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.

[github-actions[bot]]

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.

[github-actions[bot]]

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.

[evan-bradley]

I don't have the full context necessary to make an informed decision here, but I like the IsRootSpan Converter since it's clearly named and doesn't require the user to think about how that's specified in the underlying data. In particular, this statement would read well:

do_something() where IsRootSpan()

[odubajDT]

Hello! I would like to work on this issue cc @evan-bradley

[odubajDT]

To clarify the requirements:

• function IsRootSpan() accepts no input parameters and is placed in pkg/ottl
• returns true if the parent_span_id == SpanID(0x0000000000000000) - basically always when the byte array (of lenght 8) is equal zero
• returns false on all other scenarios (including parent_span_id == "" or parent_span_id == nil)

Please confirm this is the expected behavior @evan-bradley @TylerHelmuth

Thank you!

[TylerHelmuth]

@odubajDT that sounds right. This function should only be exposed for ottlspan context.

[evanderkoogh]

While having an isRootSpan() is very much easier to get than parent_span_id == SpanID(0x0000000000000000), I have spent a good chunk of time today figuring out why some of my filters were working and others weren't. And it turns out all the ones that didn't had parent_span_id == nil in them and so I started to follow those breadcrumbs and found myself here.

So I think people are going to write parent_span_id == nil because that is exactly how almost everything else works.

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.

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 SpanID function https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/5b290f02e1f9854c60c7a97598fc267962fdb44d/pkg/ottl/ottlfuncs/func_span_id.go#L34 to return nil when you pass in 0x0000000000000000, you won't break any existing parent_span_id == SpanID(0x0000000000000000) either.

[evan-bradley]

@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 nil or not.

Right now the representation has to be all zeroes because that's how Go represents the "zero value" for [8]byte. In Go, nil and []byte are also equivalent, so it wouldn't be too far-fetched to think that we could use nil here as a meaningful equivalent, while still allow interactions with empty span IDs, if it makes comparisons easier. On the other hand, I want to make sure if we deviate from the underlying data model, we won't put users in a place where they're unable to express some change to the data because we are abstracting how it's represented.

[evanderkoogh]

@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".. :)

nil and []byte might be equivalent in Go, but unfortunately the []byte is wrapped by the SpanID.

I can't quickly find out where the compare happens for SpanID, but that might also be a place to add logic. It might actually now do a check to see if the argument is an 8 length byte array and return false if it isn't. If nil would also be allowed, that might be all we need?

[evanderkoogh]

Actually.. looking at the code again, if we added a case for SpanID in values.go https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/f1b57a0911f22c936a6bb7dcc79b46d127629fa5/pkg/ottl/internal/ottlcommon/value.go#L24

And then added a case for it in compare.go: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/f1b57a0911f22c936a6bb7dcc79b46d127629fa5/pkg/ottl/compare.go#L200

We could override the behaviour to make nil and SpanID(0x0000000000000000) equivalent for comparison purposes.