open-telemetry/opentelemetry-collector

Loggers for shared components should not report signal type.

Opened this issue · 0 comments

Describe the bug

For each instantiated component, the collector assigns a set of attributes to a logger which are sufficient to identify the individual instance of the component.

When instantiating "shared" components (those which consolidate instances using e.g. a singleton pattern), this logger may contain a "signal" attribute that indicates what would have been a separate instance of the component, but is in fact representative of the shared instance. The effect is that internally generated logs are inaccurately attributed to a "metrics instance", or a "logs instance", or a "traces instance" of the component, when in reality logs were generated by an instance shared by all signals.

Steps to reproduce

Configure a single otlp receiver for metrics, logs, and traces. The collector will think that it is instantiating three separate instances. Supposing it instantiates the metrics instance first, this logger passed in will have the metrics signal as an attribute. Then, when it tries to instantiate the traces and logs instances, the otlp receiver factory will just return the same instance, which still only has the "metrics logger". For the lifecycle of the instance, it will always attribute all logs to the metrics instance.

Additionally, the order of instantiation is essentially non-deterministic, meaning that minor changes to the configuration may result in attribution to a different signal.

What did you expect to see?

The otlp receiver (and all other "shared" components) should use a logger which does not contain a signal attribute.

Potential solutions

While this issue is specifically focused on loggers, we can more broadly look at this as a question of how to ensure component instances are always using the correct set of attributes for all internally generated telemetry. In my opinion, the best solution is one that confidently hands the correct attribute set to the instance, rather than relying on component implementations to correctly follow documented guidelines.

Therefore, I suggest that we should have a simple mechanism for indicating whether or not component implementations are using a singleton pattern. This would act much like consumer.Capabilities.MutatesData, where component authors must set the flag correctly, but otherwise do not need to worry about implementation details.

Note: In theory, there could be other patterns for "shared" components, (e.g. combine logs and metrics instances, but not traces) but I'm unaware of any examples or reasons to do this. If we can assume the only valid patterns are "singleton" and "default", then a single flag is sufficient. If this doesn't seem like a safe assumption, then an enum with only two possible values would be appropriate.

Regarding the precise location for this flag - consumer.Capabilities would not work because receivers are not consumers. Therefore, I think a similar function should be added to receiver.Factory, exporter.Factory, and connector.Factory interfaces.

Additional Context

Note: I've excluded processor.Factory from the proposed solution because I believe singleton patterns should not be encouraged in processors. To understand why, say you have 2 pipelines:

pipelines:
  metrics/1:
    receivers: [ otlp/in1 ]
    processors: [ shared ]
    exporters: [otlp/out1 ]
  metrics/2:
    receivers: [ otlp/in2 ]
    processors: [ shared ]
    exporters: [otlp/out2 ]

When otlp/in1 calls shared.ConsumeMetrics, the correct behavior would be that shared will eventually call otlp/out1.ConsumeMetrics and not otlp/out2.ConsumeMetrics. However, shared.ConsumeMetrics does not have enough information to know which ConsumeMetrics to call. If it calls the wrong one or both, this constitutes a data leak across pipelines, which is not only undesirable behavior but also could cause errors if data is not fanned out with proper respect for the MutatesData flag of all downstream components.