Emit tags suitable for OpenTelemetry 1.18 Semantic Conventions

[Olamshin]

Motivation:

Ensure that spans have the right attributes to be classified correctly in an OpenTelemetry backend. OpenTelemetry conventions should override OpenTracing conventions eventually since OpenTracing has been absorbed into OpenTelemetry.

Approach:

Semantic conventions for HTTP & Messaging show some required attributes for OpenTelemetry spans. Extra tags have been added and none removed to ensure backwards compatibility. Added tags seems to be satisfactory for telemetry backends like Uptrace.

This is an upstream fix for eclipse-vertx/vertx-tracing#67

[vietj]

this seems to be a breaking change that works only for open telemetry, given the name used, can you provide a PR that works and is tested for all tracing integrations please ?

[tsegismont]

@vietj why do you think it is a breaking change? The PR only adds tags.

[Olamshin]

@vietj I believe this PR is not a breaking change, I did not change or remove tags. May I have your reasoning?

I would be opening a PR in vertx-tracing to update tests in that repo. I wanted to get buy-in with this PR and have it possibly merged. I am doing this so that I have passing tests when i open that PR.

[vietj]

@Olamshin right, then use the same convention : messaging.system -> message.system

[tsegismont]

@Olamshin right, then use the same convention : messaging.system -> message.system

I don't understand, messaging.system is a new tag in this PR, that complies with the otel specification. Which spec do you refer to with message.system?

Btw, the opentracing spec is deprecated and superseded by otel (see opentracing/specification#163)

I think we should update the comment in TagExtractor

[Olamshin]

@vietj messaging.system is specific to OpenTelemetry spec and the reason for this PR. message.system would only make sense if adhering to a Vert.x standard/spec that is not OpenTracing. OpenTracing has been deprecated and the last version of its spec does not include message.system.

[vietj]

we should keep the actual names and translate them in vertx open tracing when it get the tags. if there are missing tags then we should add them using the same convention

[vietj]

we can think about changing the tag names in Vert.x 5.0 though but we need to update the vertx tracing implementation to behave like they do now. Perhaps we should consider dropping vertx-opentracing too in 5.0

[vietj]

@vietj messaging.system is specific to OpenTelemetry spec and the reason for this PR. message.system would only make sense if adhering to a Vert.x standard/spec that is not OpenTracing. OpenTracing has been deprecated and the last version of its spec does not include message.system.

as I explained, use the same convention and translate the tag in vertx Otel implementation. We can consider using the new tag names in Vert.x 5.0

[tsegismont]

Closing, superseded by #5024

@Olamshin thank you, I've used your commit in the other PR and added modifications

[Olamshin]

@vietj messaging.system is specific to OpenTelemetry spec and the reason for this PR. message.system would only make sense if adhering to a Vert.x standard/spec that is not OpenTracing. OpenTracing has been deprecated and the last version of its spec does not include message.system.

as I explained, use the same convention and translate the tag in vertx Otel implementation. We can consider using the new tag names in Vert.x 5.0

Understood, thanks @vietj & @tsegismont!