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

Update OTLP-Jaeger translation of events according to the spec #10273

Merged
merged 2 commits into from
May 25, 2022

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented May 24, 2022

Name field of a span event was mapped to a Jaeger log field with message key which has a different meaning in OpenTracing specification: https://github.com/opentracing/specification/blob/master/semantic_conventions.md#log-fields-table.

This change updates OTLP-Jaeger translation of Span events according to the OTel Spec saying that Name field must be mapped to event Jaeger log field: https://github.com/open-telemetry/opentelemetry-specification/blob/34b907207f3dfe1635a35c4cdac6b6ab3a495e18/specification/trace/sdk_exporters/jaeger.md#events

Resolves #10268

Name field of a span event was mapped to a Jaeger log with `message` key which has a different meaning in OpenTracing specification: https://github.com/opentracing/specification/blob/master/semantic_conventions.md#log-fields-table.

This change update OTLP-Jaeger translation of Span events according to the OTel Spec saying that `Name` field should be mapped to `event` Jaeger attribute: https://github.com/open-telemetry/opentelemetry-specification/blob/34b907207f3dfe1635a35c4cdac6b6ab3a495e18/specification/trace/sdk_exporters/jaeger.md#events
@dmitryax dmitryax added the ready to merge Code review completed; ready to merge by maintainers label May 24, 2022
Comment on lines +28 to +30
// eventNameAttr is a Jaeger log field key used to represent OTel Span Event Name as defined by the OpenTelemetry Specification:
// https://github.com/open-telemetry/opentelemetry-specification/blob/34b907207f3dfe1635a35c4cdac6b6ab3a495e18/specification/trace/sdk_exporters/jaeger.md#events
const eventNameAttr = "event"
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this a featuregate protected change?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could. I just don't think this change is that impactful. WDYT? Should we?

Copy link
Member

Choose a reason for hiding this comment

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

ok... let's do it this way

@bogdandrutu bogdandrutu merged commit eea4940 into open-telemetry:main May 25, 2022
kentquirk pushed a commit to McSick/opentelemetry-collector-contrib that referenced this pull request Jun 14, 2022
…telemetry#10273)

Name field of a span event was mapped to a Jaeger log with `message` key which has a different meaning in OpenTracing specification: https://github.com/opentracing/specification/blob/master/semantic_conventions.md#log-fields-table.

This change update OTLP-Jaeger translation of Span events according to the OTel Spec saying that `Name` field should be mapped to `event` Jaeger attribute: https://github.com/open-telemetry/opentelemetry-specification/blob/34b907207f3dfe1635a35c4cdac6b6ab3a495e18/specification/trace/sdk_exporters/jaeger.md#events
axw added a commit to axw/apm-server that referenced this pull request Aug 1, 2022
Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.
axw added a commit to axw/apm-server that referenced this pull request Aug 1, 2022
Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.
axw added a commit to axw/apm-server that referenced this pull request Aug 1, 2022
Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.
axw added a commit to axw/apm-server that referenced this pull request Aug 1, 2022
Adjust code to opentelemetry-collector changes

Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events,
and OTLP logs.

open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.

open-telemetry/opentelemetry-proto#373 removed
the deprecated LogRecord.Name field. There is no replacement, hence we
no longer record `event.action`.
axw added a commit to axw/apm-server that referenced this pull request Aug 1, 2022
Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.
axw added a commit to axw/apm-server that referenced this pull request Aug 1, 2022
Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.
axw added a commit to axw/apm-server that referenced this pull request Aug 1, 2022
Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.
axw added a commit to elastic/apm-server that referenced this pull request Aug 2, 2022
* Update opentelemetry-collector modules to v0.56.0

* Adjust code to opentelemetry-collector changes

Mostly just adjusting to API changes, but there are
some functional changes related to Jaeger span events.
open-telemetry/opentelemetry-collector-contrib#10273
uncovered an issue where we were recording the "event" tag
as a label, when we should have been recording it as the
"message" field in some circumstances.

* systemtest: update otel libraries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update OTLP-Jaeger translation of events according to the spec
3 participants