feat(sdk-trace-base): add dropped attributes and events count on span

[mohitk05]

Which problem is this PR solving?

Adds two new properties on ReadableSpan:

• droppedAttributesCount: A number storing the count of dropped attributes of a span
• droppedEventsCount: A number storing the count of dropped events of a span
• droppedLinksCount: A number storing the count of dropped links of a span (currently no links are dropped, so this is always zero)

Fixes #2272

Short description of the changes

Adds two read-only objects in the Span class.

The PR updates the setAttribute and addEvent methods on Span to check for limits. When limits are reached, the counters are incremented at each occurrence of a dropped attribute/event.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Covered by unit tests.

• Should have the right count of dropped attributes/events

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added
• Documentation has been updated

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mohitk05 / name: Mohit Karekar (b2c9f40)

[dyladan]

According to spec these should be reported as span attributes, not top level items.

[codecov]

Codecov Report

Merging #3576 (474f8a9) into main (95bf6fc) will decrease coverage by 0.28%.
The diff coverage is 70.96%.

❗ Current head 474f8a9 differs from pull request most recent head 2221770. Consider uploading reports for the commit 2221770 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3576      +/-   ##
==========================================
- Coverage   93.63%   93.36%   -0.28%     
==========================================
  Files         275      257      -18     
  Lines        8095     7822     -273     
  Branches     1682     1646      -36     
==========================================
- Hits         7580     7303     -277     
- Misses        515      519       +4     

Impacted Files	Coverage Δ
...ackages/opentelemetry-sdk-trace-base/src/config.ts	88.37% <ø> (ø)
...ges/opentelemetry-exporter-jaeger/src/transform.ts	93.33% <50.00%> (-6.67%)	⬇️
...ges/opentelemetry-exporter-zipkin/src/transform.ts	90.90% <57.14%> (-9.10%)	⬇️
packages/opentelemetry-sdk-trace-base/src/Span.ts	97.38% <83.33%> (-1.20%)	⬇️
...al/packages/otlp-transformer/src/trace/internal.ts	96.77% <100.00%> (+0.22%)	⬆️
...ckages/opentelemetry-core/src/utils/environment.ts	92.53% <100.00%> (+0.22%)	⬆️
api/src/platform/node/globalThis.ts	0.00% <0.00%> (-100.00%)	⬇️
...s/opentelemetry-core/src/platform/node/sdk-info.ts	0.00% <0.00%> (-100.00%)	⬇️
...opentelemetry-core/src/platform/node/globalThis.ts	0.00% <0.00%> (-100.00%)	⬇️
...pentelemetry-core/src/platform/node/performance.ts	0.00% <0.00%> (-100.00%)	⬇️
... and 24 more

[mohitk05]

According to spec these should be reported as span attributes, not top level items.

I am using the following approach now:
Expose the dropped counts on ReadableSpan so that the non-otlp exporters can use them as attributes. In each of jaeger and zipkin exporters I've added the counts as tags as per the specification. I believe the counts have to be exposed via ReadableSpan for them to be accessible in non-otlp exporters, and then the otel.* tags should be added only to the non-otel exporters, but for OTLP, they should still be span.*count.

Let me know if this is the ideal way, or if the counts should be added as attributes directly in the mutable Span.ts (i.e. span.attributes.droppedAttributesCount vs span.droppedAttributesCount).

[dyladan]

Requesting changes because of the modified tsconfig

[dyladan]

Please add a changelog entry

[dyladan]

Dismissed my change request

[Flarna]

It's fine to do this in a followup PR. But I think we should document this somehow (maybe just an issue) that OTEL_SPAN_ATTRIBUTE_PER_EVENT_COUNT_LIMIT and OTEL_SPAN_ATTRIBUTE_PER_LINK_COUNT_LIMIT have no effect yet.

[mohitk05]

It's fine to do this in a followup PR. But I think we should document this somehow (maybe just an issue) that OTEL_SPAN_ATTRIBUTE_PER_EVENT_COUNT_LIMIT and OTEL_SPAN_ATTRIBUTE_PER_LINK_COUNT_LIMIT have no effect yet.

Sure, what would be the best way to do this?

[Flarna]

The doc of all supported env vars would be the right place but there is no such document ;o),

I think a Github issue in this repo is enough. If you plan to work on this we can assign it to you otherwise others can pick it up.

[mohitk05]

The doc of all supported env vars would be the right place but there is no such document ;o),

I think a Github issue in this repo is enough. If you plan to work on this we can assign it to you otherwise others can pick it up.

Okay, sure. I'll create an issue and can pick it up later, I do have the context so that'll help.

I have resolved the above comments, I see a couple of tests failing for me locally but they seemed unrelated. Can you trigger the workflow again?

[Flarna]

seems making droppedAttributedCount in TimedEvent results in problems so maybe best to make it optional again. Sorry.

[mohitk05]

No worries, I updated the tests to follow the type.

[mohitk05]

@Flarna I have made droppedAttributesCount optional again in TimedEvent and Link. I recalled why I had done that - since the attributes property on both these interfaces is optional, it does not make sense to make droppedAttributesCount required.

[dyladan]

I think optional is fine since it comes from the underlying proto definition

[dyladan]

This applies to more than just trace base. I think it's acceptable for the subject to apply to multiple packages.

Suggested change

	* feat(sdk-trace-base): expose dropped counts for attributes, events and links on span [#3576](https://github.com/open-telemetry/opentelemetry-js/pull/3576)
	* feat(tracing): expose dropped counts for attributes, events and links on span [#3576](https://github.com/open-telemetry/opentelemetry-js/pull/3576)

[mohitk05]

@dyladan I've resolved your comments.