Skip to content

Conversation

@andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Dec 5, 2025

Description

This PR allows users to use OTTL context inference in the routing connector. This makes routing table entries like the one below functional:

- statement: route() where span.attributes["http.method"] == "GET"
  pipelines: [traces/http]

Additionally, this PR fixes a minor bug where duplicate routing table entries were handled in the opposite manner as the error message indicated. The error message indicated that that the duplicate would be ignored, but in fact the original entry was replaced. I'm glad to extract this fix to a separate PR if that is preferred by reviewers.

Link to tracking issue

Fixes #38080

Testing

You can test the new routing features manually by downloading config.yaml which showcases the different features of this PR.

Then, run: make otelcontribcol && bin/otelcontribcol_darwin_arm64 --config config.yaml

To send test input:

# Routes to traces/demo: unqualified path defaults to resource context
telemetrygen traces --otlp-endpoint localhost:4317 --otlp-insecure --service demo-service --traces 1

# Routes to traces/prod: explicit resource context with qualified path
telemetrygen traces --otlp-endpoint localhost:4317 --otlp-insecure --otlp-attributes 'env="production"' --traces 1

# Routes to traces/default: no matching conditions
telemetrygen traces --otlp-endpoint localhost:4317 --otlp-insecure --service other-service --traces 1

# Routes to traces/http: span context inferred from qualified path
telemetrygen traces --otlp-endpoint localhost:4317 --otlp-insecure --telemetry-attributes 'http.method="GET"' --traces 1

Documentation

New docs have been added to the README

r.logger.Warn(fmt.Sprintf(`Statement %q already exists in the routing table, the route with target pipeline(s) %q will be ignored.`, item.Statement, exporters))
// Without this continue, the duplicate's pipelines would overwrite the original
// route's consumer, contradicting the warning message above.
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This continue was missing previously, meaning that the loop would not comply with the the logger warn statement above r.logger.Warn(fmt.Sprintf(Statement %q already exists in the routing table, the route with target pipeline(s) %q will be ignored., item.Statement, exporters)).

I had to restructure the control flow here because adding the continue statement triggered the lint check to request the if c {} else { continue } be restructed as an if !c { continue} ...

return err
}

// singleStatementConverter extracts a single parsed statement from the parser output.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers, if this feels like overkill I'm fine to remove it FYI

require.NoError(t, err)

statements := mockGetter{[]string{`set(attributes["bar"], "bar")`}}
_, err = pc.ParseStatementsWithContext("bar", statements, false)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no cases in the actual non-test codebase where this is set to true

// exist in resource anyway, and non-ambiguous paths (like "body", "severity_text", "name")
// only exist in one context regardless of priority. That said, the order is sorted based on which
// events are most common in practice, hence 'span' is first.
priorities := []string{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edmocosta do you have any advice for how to handle potential new types that get added to OTel in the future? I'm not nuts about this list sitting here, but it's no different than the previous case statements I suppose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[connector/routing] Using OTTL inferred context capability

2 participants