-
Notifications
You must be signed in to change notification settings - Fork 14
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
Set opentelemetry span name in transformation filter #368
Set opentelemetry span name in transformation filter #368
Conversation
Issues linked to changelog: |
if (callbacks.route() | ||
&& callbacks.route()->decorator() | ||
&& !callbacks.route()->decorator()->getOperation().empty()) { | ||
callbacks.activeSpan().setOperation(callbacks.route()->decorator()->getOperation()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO delete this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was refactored in 3d7463c
|
||
InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); | ||
std::unique_ptr<Tracing::MockSpan> mock_span = std::make_unique<Tracing::MockSpan>(); | ||
const std::unique_ptr<Router::MockDecorator> mock_decorator = std::make_unique<NiceMock<Router::MockDecorator>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there simpler ways to set up these mocks?
This is the default value that is set from the HCM - we do not need to write it a second time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested it out on my local as well. I like it!
* Add setOperation patch * Add changelog * Transformation initial implementation Should work but does not seem to at the moment * Implement unit test for setting span name * Update changelog * Remove extra entry from changelog * Do not override route.decorator.operation * Add test for null route in callbacks * Remove redundant call to setOperation This is the default value that is set from the HCM - we do not need to write it a second time.
…368) (#369) * Set opentelemetry span name in transformation filter (#368) * Add setOperation patch * Add changelog * Transformation initial implementation Should work but does not seem to at the moment * Implement unit test for setting span name * Update changelog * Remove extra entry from changelog * Do not override route.decorator.operation * Add test for null route in callbacks * Remove redundant call to setOperation This is the default value that is set from the HCM - we do not need to write it a second time. * Move changelog * Move changelog... again!
This pull request adds the ability to set the span name in the transformation filter.
We add a new field in the transformation template to allow setting modifying the span. The new field currently only accepts the span name as a field, but attributes/baggage/other data could also be added as fields to be modified here as well.
If
route.decorator.operation
is set, we should not update the field in this filter - we intend for the operation field to take precedence over this value, which is set at the virtual host level and intended to be used as a default instead.Here is an envoy configuration for testing - note the presence of
route.decorator.operation
, and how uncommenting it causes Envoy to use that field as the span name instead of the hostname.