Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Processors should remove SchemaURL when transformations violate semantic conventions #19472

Closed
jsuereth opened this issue Mar 13, 2023 · 6 comments
Labels
bug Something isn't working closed as inactive processor/attributes Attributes processor processor/cumulativetodelta Cumulative To Delta processor processor/deltatorate Delta To Rate processor processor/logstransform Logs Transform processor processor/metricstransform Metrics Transform processor processor/resource Resource processor processor/transform Transform processor Stale

Comments

@jsuereth
Copy link
Contributor

Component(s)

processor/attributes, processor/cumulativetodelta, processor/deltatorate, processor/groupbyattrs, processor/logstransform, processor/metricstransform, processor/resource, processor/transform

What happened?

Description

When using a processor to alter telemetry that has a valid Telemetry Schema URL, it's possible/likely that the processor could cause the telemetry to violate the associated schema.

Steps to Reproduce

Telemetry now violates the schema

Expected Result

The renamed span should be in a new InstrumentationScope that does NOT include a SchemaURL.

Actual Result

The renamed span remains in an InstrumentationScope that declares compatibility with 1.20.0 of the semantic conventions.

Collector version

v0.73.0

Environment information

Environment

The otel-collector-contrib docker image

OpenTelemetry Collector configuration

receivers:
  otlp:
exporters:
  logging:
processors:
  attributes/bad:
    actions:
      - key: net.app.protocol.name
        action: insert
        from_context: net.protocol.name
      - key: net.protocol.name
        action: delete
pipelines:
  traces:
    receivers: [otlp]
    processors: [attributes/bad]
    exporters: [logging]

Log output

No response

Additional context

This is targeted at the viability of SchemaURL itself and its enforceability through the ecosystem.

  • Can we simplify processing telemetry so SchemaURL is dropped whenever its guarantees would be violated?
  • Do we have the right tools in place to determine, detect and enforce this?
  • What does the collector need to make this portion of the specification as easy as possible to deal with?

cc @tigrannajaryan

@jsuereth jsuereth added bug Something isn't working needs triage New item requiring triage labels Mar 13, 2023
@github-actions github-actions bot added processor/attributes Attributes processor processor/cumulativetodelta Cumulative To Delta processor processor/deltatorate Delta To Rate processor processor/logstransform Logs Transform processor processor/metricstransform Metrics Transform processor processor/resource Resource processor processor/transform Transform processor labels Mar 13, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

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

@atoulme atoulme removed the needs triage New item requiring triage label Mar 13, 2023
@TylerHelmuth
Copy link
Member

I'm not sure enforcing something like this is something we could, or want to, make each processor do. The minutia of the specification was written with the APIs/SDKs in mind, not the Collector, and as a result it doesn't always fit. I see processing data in the collector to be one of those places.

I'd rather the collector be the one tool OTel users can use to process their data how they want to. Yes that allows them the get into non-spec compliant or confusing situations, but I think giving the tools is still important. The collector already allows for other non-spec compliant transformations such as orphaned telemetry, unsound transformations (like not re-aggregating metrics, etc). I think this situation is another meaningful one to add to the list.

Instead of enforcing, I would vote for adding another Warning and continuing work on #19172.

@tigrannajaryan
Copy link
Member

I don't think it is possible to automatically deduce whether a particular modification results in violating the semantic conventions. The semantic convention specification lacks the necessary formally and expressiveness for such checks to be possible to write in code.

One possibility I see is that we mark every modification as potentially violating the conventions, but I am not sure it is very useful.

Absent some significant rethinking in how the conventions are defined in the spec I don't see how this issue can be implemented.

@jsuereth
Copy link
Contributor Author

The laziest possible thing to do is just ALWAYS remove SchemaURL on any change to telemetry.

I'd prefer not to do that.

Possibly we add a generic config for processors where users can optionally write preserve_schema_url: false ?

@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot added the Stale label May 22, 2023
@github-actions
Copy link
Contributor

This issue has been closed as inactive because it has been stale for 120 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working closed as inactive processor/attributes Attributes processor processor/cumulativetodelta Cumulative To Delta processor processor/deltatorate Delta To Rate processor processor/logstransform Logs Transform processor processor/metricstransform Metrics Transform processor processor/resource Resource processor processor/transform Transform processor Stale
Projects
None yet
Development

No branches or pull requests

4 participants