New AdjustedCount converter function

[lahsivjar]

Component(s)

pkg/ottl

Is your feature request related to a problem? Please describe.

Adjusted count provides a measure of how many spans an individually sampled span represents. It would be nice to have a function in OTTL that could calculate the adjusted count for a span.

Describe the solution you'd like

Add a new AdjustedCount converter function. The function will be applicable only for spans (similar to IsRootSpan function). The adjusted count can be calculated by using the sampling pkg, something like:

import (
	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/sampling"
)

func CalculateAdjustedCount(tracestate string) float64 {
	w3cTraceState, err := sampling.NewW3CTraceState(tracestate)
	if err != nil {
		return 1
	}
	otTraceState := w3cTraceState.OTelValue()
	if otTraceState == nil {
		return 1
	}
	if len(otTraceState.TValue()) == 0 {
		// For non-probabilistic sampler OR always sampling threshold, default to 1
		return 1
	}
	return otTraceState.AdjustedCount()
}

Describe alternatives you've considered

No response

Additional context

No response

[github-actions]

Pinging code owners:

• pkg/ottl: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[lahsivjar]

Created a PR to show how this looks like: #36573

[TylerHelmuth]

@lahsivjar OTTL has access to the trace_state, and trace_state[""], is there a way to calculate this value using the trace_state and OTTL's arithmetic capabilities?

If not adding support for this otel feature is appropriate. I wonder tho if a converter or a new path segment is better. We could add to the ottlspan context the idea of trace_state.adjusted_count to get the value.

@evan-bradley what do you think?

[evan-bradley]

I agree, trace_state.adjusted_count makes more sense since there are no parameters and we're essentially getting a modified version of a specific path.

At a quick glance of the pkg/sampling package the PR uses, the functionality for the adjusted count also appears to be something that is better handled by a library instead of in OTTL, so I think it's okay to add an explicit path for it.

[kentquirk]

I believe it's worth having, and should use pkg/sampling, as the math is slightly tricky. Even if OTTL could do it, this is important to get right.

I have a couple of cautions:

1. OTEP 235 has not landed yet at the W3C, so the spec isn't completely finalized.
2. The API in pkg/sampling is therefore not yet final (but it's probably pretty close).
3. If we're going to introduce AdjustedCount we should also consider introducing at least SamplingProbability and possibly SamplingThreshold at the same time, since people seem to have strong preferences as to which they need.

[lahsivjar]

I believe it's worth having, and should use pkg/sampling, as the math is slightly tricky. Even if OTTL could do it, this is important to get right.

Agreed on using the pkg/sampling, I initially tried using OTTL expression to do this but in addition to it being complicated to reason/maintain the OTTL expression can get outdated if the spec around sampling evolves.

I agree, trace_state.adjusted_count makes more sense since there are no parameters and we're essentially getting a modified version of a specific path.

Introducing this as a path also sounds good to me, and, IIANM, we could still use the pkg/sampling to do this.

OTEP 235 has not landed yet at the W3C, so the spec isn't completely finalized.
The API in pkg/sampling is therefore not yet final (but it's probably pretty close).

How do we deal with this? Are there any concerns about evolving/introducing breaking changes in the OTTL implementation if the spec changes?

If we're going to introduce AdjustedCount we should also consider introducing at least SamplingProbability and possibly SamplingThreshold at the same time, since people seem to have strong preferences as to which they need.

ACK, I have planned to work on at least the SamplingProbability as a follow-up but I can also include the SamplingThreshold. Going back to the implementation, should we keep these as converters, or are paths more suitable? I don't have a strong inclination either way but I don't have in-depth knowledge about OTTL grammar -- the points presented in #36572 (comment) sound reasonable to me though.

[evan-bradley]

Going back to the implementation, should we keep these as converters, or are paths more suitable?

I would still like to stick to implementing these as paths. Converters are usually for converting arbitrary input data to some output format. In this case, we're looking to access data on a specific location on the span.

[lahsivjar]

I would still like to stick to implementing these as paths

I have a draft PR for adding trace_state.adjusted_count but I am having second thoughts on the path approach. AFAIU, the paths seem to have both getters and setters and it makes sense since they usually refer directly to something that can be accessed. However, in the case of adjusted count, it is a derived value from a trace state and, given an adjusted count, we cannot update the trace state (at least based on my understanding but CMIIW).

[TylerHelmuth]

We support some other get-only paths (metric type), it is ok if we don't implement a set for the path if setting an adjusted_count is a meaningless/undefined concept.