open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
2.88k stars 2.26k forks source link

[pkg/stanza] Clarify how "nested" attributes produced by operators should be handled #33029

Open ChrsMark opened 3 months ago

ChrsMark commented 3 months ago

Component(s)

pkg/stanza

Describe the issue you're reporting

As discussed at https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32594#issuecomment-2107540529, it's not super clear if attributes produced by stanza operators should be nested of flat.

A configuration like the following:

receivers:
  filelog:
    include:
    - /var/log/pods/*/*/*.log
    operators:
      - id: some
        type: add
        field: attributes.key2.key_in
        value: val2

gives:

Attributes:
     ...
     -> key2: Map({"key_in":"val2"})

If we want to have a flat field instead of a nested one we should use field: attributes["key2.key_in"].

This can be confusing for users (and even component developers) so this issue is to file this "inconsistency" and explore possible solutions.

2 first ideas that come to mind: A) make the field: attributes.key2.key_in to produce flat attributes as well B) in case we want to keep both, or we cannot change the behavior we should document the difference so that users know what to expect specifically.

github-actions[bot] commented 3 months ago

Pinging code owners:

djaglowski commented 3 months ago

I think the attributes should definitely be flat, but there is an open question as to whether we should change the stanza's field syntax.

Currently, attributes.foo.bar is used as shorthand for attributes["foo"]["bar"]. It is also possible to unambiguously define attributes["foo.bar"].

I am in favor of making the change but would like to hear from others. If we move forward with it, I think we should have a reasonably long deprecation process with clear warnings.

andrzej-stencel commented 3 months ago

If I understand correctly, the current state does not prevent users from doing what they want, right? They can use the notation attributes["foo.bar"] to specify flat attributes. Perhaps we could save the users a breaking change and just leave things as they are, adding documentation to prevent confusion?

djaglowski commented 3 months ago

Correct, there's no limitation, just inconsistency between the meaning of attributes.foo.bar here vs other places. I'm fine leaving it or aligning it with a long deprecation process. Would appreciate more opinions @open-telemetry/collector-contrib-approvers

github-actions[bot] commented 1 month ago

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 @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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