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

Standardize how to record DroppedAttributeCount in protocols that have no equivalent #1656

Closed
tigrannajaryan opened this issue Apr 27, 2021 · 4 comments · Fixed by #1662
Closed
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:protocol Related to the specification/protocol directory

Comments

@tigrannajaryan
Copy link
Member

tigrannajaryan commented Apr 27, 2021

For Jaeger we suggest to use otel.event.dropped_attributes_count tag for Events (Jaeger Logs). We say nothing about how to record the value of DroppedAttributeCount for Spans.

For Zipkin this is not defined (the spec says TBD).

I believe we need one standard way to record DroppedAttributeCount in protocols that have no equivalent and do it the same way for Spans, SpanEvents, Logs, Records.

I suggest that we always use otel.dropped_attributes_count attribute name to represent DroppedAttributeCount field of Spans, SpanEvents, Logs, Records when the equivalent does not exist in the protocol.

@tigrannajaryan tigrannajaryan added the spec:protocol Related to the specification/protocol directory label Apr 27, 2021
@tigrannajaryan tigrannajaryan self-assigned this Apr 27, 2021
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Apr 29, 2021
Fixes open-telemetry#1656

The documents that describe transformations to Jaeger and Zipkin contained
duplicate information about how to record InstrumentationLibrary.

They also contained incomplete and incorrect information about how to record
dropped attribute count.

This change extracts common transformation rules into a separate document (non-otlp.md)
and references this document where appropriate.

We also define otel.event.dropped_attributes_count as the only way to record
dropped attribute count, regardless of the entity for which it is recorded.
Previously in Jaeger format we specified that for Events we must use
otel.event.dropped_attributes_count and did not specify at all what to use for Spans.

I believe using the same uniform key for Spans and Events is correct.

I consider this a specification bug and do not believe it should be treated as
a breaking change.

Also delete the Zipkin mapping for LocalChildSpanCount since there is no such
OpenTelemetry field anymore.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Apr 29, 2021
Fixes open-telemetry#1656

The documents that describe transformations to Jaeger and Zipkin contained
duplicate information about how to record InstrumentationLibrary.

They also contained incomplete and incorrect information about how to record
dropped attribute count.

This change extracts common transformation rules into a separate document (non-otlp.md)
and references this document where appropriate.

We also define otel.event.dropped_attributes_count as the only way to record
dropped attribute count, regardless of the entity for which it is recorded.
Previously in Jaeger format we specified that for Events we must use
otel.event.dropped_attributes_count and did not specify at all what to use for Spans.

I believe using the same uniform key for Spans and Events is correct.

I consider this a specification bug and do not believe it should be treated as
a breaking change.

Also delete the Zipkin mapping for LocalChildSpanCount since there is no such
OpenTelemetry field anymore.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Apr 29, 2021
Fixes open-telemetry#1656

The documents that describe transformations to Jaeger and Zipkin contained
duplicate information about how to record InstrumentationLibrary.

They also contained incomplete and incorrect information about how to record
dropped attribute count.

This change extracts common transformation rules into a separate document (non-otlp.md)
and references this document where appropriate.

We also define otel.event.dropped_attributes_count as the only way to record
dropped attribute count, regardless of the entity for which it is recorded.
Previously in Jaeger format we specified that for Events we must use
otel.event.dropped_attributes_count and did not specify at all what to use for Spans.

I believe using the same uniform key for Spans and Events is correct.

I consider this a specification bug and do not believe it should be treated as
a breaking change.

Also delete the Zipkin mapping for LocalChildSpanCount since there is no such
OpenTelemetry field anymore.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Apr 29, 2021
Fixes open-telemetry#1656

The documents that describe transformations to Jaeger and Zipkin contained
duplicate information about how to record InstrumentationLibrary.

They also contained incomplete and incorrect information about how to record
dropped attribute count.

This change extracts common transformation rules into a separate document (non-otlp.md)
and references this document where appropriate.

We also define otel.event.dropped_attributes_count as the only way to record
dropped attribute count, regardless of the entity for which it is recorded.
Previously in Jaeger format we specified that for Events we must use
otel.event.dropped_attributes_count and did not specify at all what to use for Spans.

I believe using the same uniform key for Spans and Events is correct.

I consider this a specification bug and do not believe it should be treated as
a breaking change.

Also delete the Zipkin mapping for LocalChildSpanCount since there is no such
OpenTelemetry field anymore.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Apr 30, 2021
Fixes open-telemetry#1656

The documents that describe transformations to Jaeger and Zipkin contained
duplicate information about how to record InstrumentationLibrary.

They also contained incomplete and incorrect information about how to record
dropped attribute count.

This change extracts common transformation rules into a separate document (non-otlp.md)
and references this document where appropriate.

We also define otel.event.dropped_attributes_count as the only way to record
dropped attribute count, regardless of the entity for which it is recorded.
Previously in Jaeger format we specified that for Events we must use
otel.event.dropped_attributes_count and did not specify at all what to use for Spans.

I believe using the same uniform key for Spans and Events is correct.

I consider this a specification bug and do not believe it should be treated as
a breaking change.

Also delete the Zipkin mapping for LocalChildSpanCount since there is no such
OpenTelemetry field anymore.
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this issue Apr 30, 2021
Fixes open-telemetry#1656

The documents that describe transformations to Jaeger and Zipkin contained
duplicate information about how to record InstrumentationLibrary.

They also contained incomplete and incorrect information about how to record
dropped attribute count.

This change extracts common transformation rules into a separate document (non-otlp.md)
and references this document where appropriate.

We also define otel.event.dropped_attributes_count as the only way to record
dropped attribute count, regardless of the entity for which it is recorded.
Previously in Jaeger format we specified that for Events we must use
otel.event.dropped_attributes_count and did not specify at all what to use for Spans.

I believe using the same uniform key for Spans and Events is correct.

I consider this a specification bug and do not believe it should be treated as
a breaking change.

Also delete the Zipkin mapping for LocalChildSpanCount since there is no such
OpenTelemetry field anymore.
@reyang reyang added the area:semantic-conventions Related to semantic conventions label May 4, 2021
@reyang
Copy link
Member

reyang commented May 4, 2021

@reyang to follow up with @carlosalberto and create milestones for tracking.

tigrannajaryan added a commit that referenced this issue May 4, 2021
…1662)

Fixes #1656

The documents that describe transformations to Jaeger and Zipkin contained
duplicate information about how to record InstrumentationLibrary.

They also contained incomplete and incorrect information about how to record
dropped attribute count.

This change extracts common transformation rules into a separate document (non-otlp.md)
and references this document where appropriate.

We also define otel.event.dropped_attributes_count as the only way to record
dropped attribute count, regardless of the entity for which it is recorded.
Previously in Jaeger format we specified that for Events we must use
otel.event.dropped_attributes_count and did not specify at all what to use for Spans.

I believe using the same uniform key for Spans and Events is correct.

I consider this a specification bug and do not believe it should be treated as
a breaking change.

Also delete the Zipkin mapping for LocalChildSpanCount since there is no such
OpenTelemetry field anymore.
@codeboten
Copy link
Contributor

codeboten commented May 13, 2021

@tigrannajaryan it's unclear to me what generates these dropped attributes/events in the first place. Is that defined somewhere in the specification?

To be clear, I'm asking what would be causing this value to be non-zero

This key-value pair should only be recorded when it contains a non-zero value.

@tigrannajaryan
Copy link
Member Author

@tigrannajaryan it's unclear to me what generates these dropped attributes/events in the first place. Is that defined somewhere in the specification?

To be clear, I'm asking what would be causing this value to be non-zero

I believe they should be populated by SDKs when the limits are hit: https://github.com/open-telemetry/opentelemetry-specification/blob/5a19b53d71e967659517c02a69b801381d29bf1e/specification/trace/sdk.md#span-limits

It is not defined explicitly in the spec, worth adding.

@codeboten
Copy link
Contributor

Thank you for clarifying! I can add this to the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:protocol Related to the specification/protocol directory
Projects
None yet
3 participants