System for managing own telemetry attributes within pipeline components

[djaglowski]

Some types of components are fully or partially shared across multiple instances. (e.g. otlp receiver / memory limiter)

I have previously pursued some optimistic models in which the collector would define certain ways in which sharing is "supported", but the reality is that sharing is an unbounded problem. e.g. Someone could write a processor that shares a data structure between logs and traces pipelines only if it's instantiated on a Wednesday and the component name is "foo". I am convinced that there are too many novel cases to model ahead of time, so here is a proposal to normalize attributes as much as possible, while still allowing component authors to both add or subtract attributes from their telemetry when appropriate.

I will use Loggers as the example here, but I believe the same general ideas can be applied to TracerProviders and MeterProviders.

---

General Idea:

Create a "Logger" struct specifically for use within components. This logger satisfies the same interface and behaves essentially the same as what we have today. However, it has the following important differences:

1. The attributes are preset for the component as if it uses no sharing of parts with other instances of the component. (As defined in this RFC)
2. A Without(key ...string) method is provided, which allows component authors to remove attributes as appropriate for portions of the code that are shared. This is opposite the typical pattern used by loggers, but necessary if we want our default set of attributes to be correct.

For example:

• OTLP receiver: Since the entire component struct is shared, the factory will use Without("otelcol.signal") during construction to create a logger appropriate for the shared instance.
• Memory limiter processor: For any code outside of the shared part, it can use the logger which it was given. For the shared part, is can create a new logger by calling Without("otelcol.signal", "otelcol.component.id", "otelcol.pipeline.id").
• Custom component mentioned above: Typically uses the given logger, but if creating a logs or traces instance on a Wednesday and the component name is "foo", then it calls Without("otelcol.signal", "otelcol.component.id") to adjust the scope.

---

This implementation looks something like this:

import (
	"go.opentelemetry.io/otel/attribute"
	"go.uber.org/zap"
)

type ComponentLogger struct {
	*zap.Logger
	root *zap.Logger
	diff []attribute.KeyValue
}

func NewComponentLogger(root *zap.Logger, defaultAttrs map[string]string) *ComponentLogger {
	defaultLogger := root
	for k, v := range defaultAttrs {
		defaultLogger = defaultLogger.With(zap.String(k, v))
	}

	return &ComponentLogger{
		Logger: defaultLogger,
		root:   root,
		diff:   defaultAttrs,
	}
}

func (cl *ComponentLogger) Without(keys ...string) *zap.Logger {
	excludeKeys := make(map[string]struct{})
	for _, key := range keys {
		excludeKeys[key] = struct{}{}
	}

	logger := cl.root
	for _, attr := range cl.diff {
		if _, excluded := excludeKeys[string(attr.Key)]; !excluded {
			logger = logger.With(zap.String(k, v))
		}
	}
	return logger
}

func (cl *ComponentLogger) With(fields ...zap.Field) *ComponentLogger {
	// Create new diff by appending the new fields as attributes
	newDiff := make([]attribute.KeyValue, len(cl.diff))
	copy(newDiff, cl.diff)
	for _, f := range fields {
		newDiff = append(newDiff, attribute.Any(f.Key, f.Interface))
	}

	return &ComponentLogger{
		Logger: cl.Logger.With(fields...),
		root:   cl.root,
		diff:   newDiff,
	}
}

---

TracerProvider and MeterProvider can expose a similar With/Without interface, but the implementation will be different.

[dmitryax]

I like the general idea. A couple of questions:

• Is there a need to have With method at this point?
• Requiring string arguments for Without might be error-prone. Yes, we can provide public constants. But maybe we can have more specific methods, e.g., WithSharedSignal(), instead?

[djaglowski]

Is there a need to have With method at this point?

We can leave it out. There may be some uses out there already, but they would be able to access it on the embedded zap logger.

Requiring string arguments for Without might be error-prone. Yes, we can provide public constants. But maybe we can have more specific methods, e.g., WithSharedSignal(), instead?

We have 5 different attributes defined in the RFC, which differ based on component kind. Additionally, I would hope that in practice we see the need for removing attributes only a few specific cases. So my opinion is that such methods would be either inapplicable (e.g. receiver doesn't have a otelcol.pipeline.id to remove) or unadvisable in most cases. That said, I think limiting the values to those we enumerate makes things less error prone, so that would be my preference.

[djaglowski]

I think we need a new module which eventually contains:

• Attribute keys agreed upon in the RFC
• The logger described above
• Similar meter & tracer providers

Constraints:

• Component implementations will use the package, so cannot be in service.
• Should be able to construct appropriate attribute sets from e.g. pipeline.Signal and component.ID, so cannot be in component. Seems out of place in pipeline too though.

The best I can come up with is component/componentattribute. Does this seem reasonable @open-telemetry/collector-maintainers, or do you have other suggestions?

[djaglowski]

Draft solution: #12259

[mx-psi]

The best I can come up with is component/componentattribute. Does this seem reasonable @open-telemetry/collector-maintainers, or do you have other suggestions?

Sounds reasonable to me

[dmitryax]

We have 5 different attributes defined in the RFC, which differ based on component kind. Additionally, I would hope that in practice we see the need for removing attributes only a few specific cases. So my opinion is that such methods would be either inapplicable (e.g. receiver doesn't have a otelcol.pipeline.id to remove) or unadvisable in most cases. That said, I think limiting the values to those we enumerate makes things less error prone, so that would be my preference.

We have the solution for the logger now. However, we will likely need to apply a similar approach to the meter and tracer in the future. If we keep adding Without to them, it'll be hard for API users to catch up and keep it consistent. Especially when we need to replace data_type metrics attribute with otelcol.signal (probably via a feature gate). Having something like WithSharedSIgnal would help to consolidate it across the signals and centralize such migrations if needed. I don't believe we need dropping any other attributes at this point. Once we have such a need, a new method could be added, even if it's not useful in some scenarios.

Also, I think it could be one mutation call to TelemetrySettings instead of changing logger, meter and tracer separately. Something like TelemetrySettings.SharedSignal would return another instance of TelemetrySettings with logger, meter, and tracer not having that attribute.

[mx-psi]

Also, I think it could be one mutation call to TelemetrySettings instead of changing logger, meter and tracer separately. Something like TelemetrySettings.SharedSignal would return another instance of TelemetrySettings with logger, meter, and tracer not having that attribute.

TelemetrySettings does not have a meter and a tracer, it has a meter provider and a tracer provider. These do not have any attributes set

[jade-guiton-dd]

The meter/tracer provider don't contain attributes themselves, so to propagate the component attributes we would probably want to add separatetrace.SpanStartEventOption and metric.MeasurementOption fields to TelemetrySettings (or just a []attribute.KeyValue to be wrapped in options later). In that context, I think having a mutable method on TelemetrySettings to subtract attribute keys for all three signals at once would be convenient.

I do however think the Without approach is more flexible then WithSharedSignal or something similarly specific; @djaglowski's PR already included a use case for Without("otelcol.component.id") in the form of the memorylimiterprocessor.

[djaglowski]

Since MeterProvider and TracerProvider are interfaces, my plan was to wrap them into new implementations of their respective interfaces, where the new implementations have default attributes that can be removed by component authors using Without.

Am I understanding correctly, that you'd like us to have a single Without method which updates the TelemetrySettings as a whole, so that the Logger, MeterProvider and TracerProvider all have the same attributes modified? This seems reasonable to me.

Having something like WithSharedSIgnal would help to consolidate it across the signals and centralize such migrations if needed.

I don't think it's necessary vs just listing constants, e.g., in memory limiter:

1. telSet = telSet.Without(semconv.OTelColComponentID, semconv.OTelColPipelineID, semconv.OTelColSignal)
2. telSet = telSet.WithSharedComponentID().WithSharedPipelineID().WithSharedSignal()
3. telSet = telSet.WithShared(WithSharedComponentID(), WithSharedPipelineID(), WithSharedSignal())

Minor notes:

• (2) and maybe (3) require recreation of the TelemetrySettings multiple times.
• If (3), is there a better name for the method?

Personally I like (1) best as I believe component authors should be thinking about the attributes on their telemetry, rather than thinking about an abstraction which they may not recognize as impacting their telemetry.

[mx-psi]

Since MeterProvider and TracerProvider are interfaces, my plan was to wrap them into new implementations of their respective interfaces, where the new implementations have default attributes that can be removed by component authors using Without.

If the intent is to use metric-level attributes for this, I suspect it will be a bit complex to do this for MeterProvider, given the number of instruments that you would have to wrap and because of the unfortunate quirks of the spec. I am not saying that we shouldn't do it because of that, just that it is probably harder than it looks.

Since the discussion includes the possibility of modifying the recently added method LoggerWithout I marked this as release:blocker (we can always just remove this method before the release if we do not reach consensus).

[mx-psi]

I tried doing the MeterProvider wrapper: you need at least 21 wrapper structs (MeterProvider + Meter + each instrument + the observables for async instruments) to make it work. We could do it if we want to, it just makes me feel like this is not the right solution, given how complicated this is.

[mx-psi]

Concretely: I wonder if metric attributes are not the right level for this, and these should be instrumentation attributes or resource attributes. The RFC is not very clear about what type of attributes these should be.

[dmitryax]

Assuming we can find a clean way to move towards this approach and avoid overcomplication, I would prefer the option (3)

telSet = telSet.WithShared(WithSharedComponentID(), WithSharedPipelineID(), WithSharedSignal())

It'll help us control the migration of some attribute keys (e.g., data_type -> otelcol.signal) and make the interface less error-prone.

Concretely: I wonder if metric attributes are not the right level for this, and these should be instrumentation attributes or resource attributes. The RFC is not very clear about what type of attributes these should be.

We need to think about what the resource is in our case (which entity). We've been considering the collector as the resource. If we want to move some attributes to the resource level, we need to reconsider this. I think we can move all the attributes to the resource level if we consider the resource to be component instance. However, this can complicate re-aggregation. Currently, the collector doesn't provide any processor to do aggregation by resource attributes as far as I know, only by the datapoint attributes . So I'm not sure it that's the right move, but we still can consider it if that's required to proceed with the RFC.

[djaglowski]

I tried doing the MeterProvider wrapper: you need at least 21 wrapper structs (MeterProvider + Meter + each instrument + the observables for async instruments) to make it work. We could do it if we want to, it just makes me feel like this is not the right solution, given how complicated this is.

Since the library we are wrapping is stable, this seems reasonable to me. However, I think it's worth clarifying first whether we want to model component instances as resources or entities. IMO instrumentation is not technically correct because we intend to have external code (e.g. in the service graph) produce signals describing the component instances, but not originating from within them.

[jade-guiton-dd]

My impression is that resource attributes describe where the telemetry is coming "from" as well, so I don't think it would be correct either. I'm not sure there is a convention for instrumentation libraries (here, the pipeline) to indicate which user (a component) of the instrumented library (the Consumer API?) caused the observed event. My opinion is that perhaps the least bad solution would be to set the instrumentation library name to go.opentelemetry.io/collector/service or something similar, but use instrumentation attributes to describe the instrumented component?

[mx-psi]

@djaglowski could we add the clarification of what kind of attributes these are to the RFC? I don't have a strong opinion, but I think we can close the discussion that way.

Also, @djaglowski @dmitryax I filed #12334 so that we can keep on experimenting on this while being able to mark component as 1.0

[djaglowski]

could we add the clarification of what kind of attributes these are to the RFC? I don't have a strong opinion, but I think we can close the discussion that way.

I agree we should update the RFC but I'd like to have some idea of what to propose. I'm unclear myself on what type of attributes are reasonable.

My impression is that resource attributes describe where the telemetry is coming "from" as well, so I don't think it would be correct either.

Resource attributes don't necessarily have to be applied from within the same code that the signals describe. For example, most pull-based receivers attribute signals to a resource, despite the receiver not being part of that resource.

---

If we were to say that the RFC describes resource attributes, how is this applied for loggers?