feat(sdk-logs)!: implement scope attributes for logger creation

[pichlermarc]

Which problem is this PR solving?

The spec defines a concept called scope attributes - another level of attribute that's attached to the scope instead of the resource or log records.

To prepare for the upcoming Logs SDK/API GA, this PR:

• implements scopeAttributes support in LoggerProvider#getLogger() ([sdk-logs] implement scopeAttributes #6413)
• implements scopeAttributes support in the OTLP export pipeline ([sdk-logs] implement scopeAttributes #6413)

Note on performance: With the changes proposed in this PR, use of scopeAttributes with LoggerProvider#getLogger() has a noticeable performance impact as attributes contribute to the Logger's identity. I decided to implement it this way, as it allows us to front-load the performance impact in a way that's easy the end-user to control as well as predictable regardless of which exporter is used.

If we were not do it this way, there would be no efficient way to sort and batch by scope in the exporters, which would require us to perform the same operation numerous times to de-dupe scopes - this would have a guaranteed performance impact on each export cycle. Creating Loggers should not be something that happens on a hot-path - the added API documentation also now warns the user that LoggerProvider#getLogger() may be an expensive operation.

Disclosure of AI use: I used Claude Haiku 4.5 for generating small bits and pieces of this PR, mostly tests and docs.

Fixes #6413

Type of change

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

• existing tests
• added tests

[codecov]

Codecov Report

❌ Patch coverage is 97.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.85%. Comparing base (66499ce) to head (11af96c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...rimental/packages/sdk-logs/src/utils/validation.ts	95.65%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6573   +/-   ##
=======================================
  Coverage   94.85%   94.85%           
=======================================
  Files         377      377           
  Lines       12751    12816   +65     
  Branches     2887     2913   +26     
=======================================
+ Hits        12095    12157   +62     
- Misses        656      659    +3     

Files with missing lines	Coverage Δ
experimental/packages/api-logs/src/api/logs.ts	100.00% <ø> (ø)
...l/packages/otlp-transformer/src/common/internal.ts	100.00% <100.00%> (ø)
...ansformer/src/common/protobuf/common-serializer.ts	98.33% <100.00%> (+0.07%)	⬆️
...tal/packages/otlp-transformer/src/logs/internal.ts	100.00% <100.00%> (ø)
.../packages/otlp-transformer/src/metrics/internal.ts	100.00% <ø> (ø)
...al/packages/otlp-transformer/src/trace/internal.ts	98.55% <ø> (ø)
...xperimental/packages/sdk-logs/src/LogRecordImpl.ts	99.12% <100.00%> (-0.20%)	⬇️
experimental/packages/sdk-logs/src/Logger.ts	100.00% <100.00%> (ø)
...perimental/packages/sdk-logs/src/LoggerProvider.ts	100.00% <100.00%> (ø)
...perimental/packages/sdk-logs/src/internal/utils.ts	100.00% <100.00%> (ø)
... and 1 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pichlermarc]

note for reviewers: by avoding this string operation we get a significant performance boost for JSON serialization. we already do this in the protobuf serializer for performance reasons. Having to the string operation for each log record with scopeAttributes would be quite expensive.

Old:

transform 512 logs (json) x 734 ops/sec ±0.33% (97 runs sampled)

New:

transform 512 logs (json) x 770 ops/sec ±0.78% (92 runs sampled)

[trentm]

Initial feedback.

[trentm]

Whether to incr droppedAttributesCount for invalid keys.
The proto says:
https://github.com/open-telemetry/opentelemetry-proto/blob/aca87357903c668e441718a144be0adf435afec1/opentelemetry/proto/common/v1/common.proto#L116-L119

  // The number of attributes that were discarded. Attributes
  // can be discarded because their keys are too long or because there are too many
  // attributes. If this value is 0, then no attributes were dropped.
  uint32 dropped_attributes_count = 4;

which suggests maybe not increment for invalid given keys.

The current LogRecord.setAttribute() does not incr for invalid keys:

opentelemetry-js/experimental/packages/sdk-logs/src/LogRecordImpl.ts

Lines 128 to 143 in 87d0112

	public setAttribute(key: string, value?: AnyValue) {
	if (this._isLogRecordReadonly()) {
	return this;
	}
	if (key.length === 0) {
	api.diag.warn(`Invalid attribute key: ${key}`);
	return this;
	}
	if (!isLogAttributeValue(value)) {
	api.diag.warn(`Invalid attribute value set for key: ${key}`);
	return this;
	}
	const isNewKey = !Object.prototype.hasOwnProperty.call(
	this.attributes,
	key
	);

Ultimately there should be one shared utility that handles applying (a) validation (that given attribute values are of supported types) and (b) limits.

I am fine refining this over a couple of PRs, however. I.e. doesn't have to be in this PR.

[pichlermarc]

Hmm interesting, the comment in the proto looked to me like it's giving examples of what could lead to dropped attributes, not an exhaustive list.

The spec mentions this about what makes an attributes valid/invalid:

An Attribute is a key-value pair, which MUST have the following properties:

• The attribute key MUST be a non-null and non-empty string.
  • Case sensitivity of keys is preserved. Keys that differ in casing are treated as distinct keys.
• The attribute value MUST be one of types defined in AnyValue.

The current LogRecord.setAttribute() does not incr for invalid keys

Hmm, I think we should increment since there is data that's potentially being dropped. I'll check what other SDKs are doing and will report back here.

Ultimately there should be one shared utility that handles applying (a) validation (that given attribute values are of supported types) and (b) limits.

Yep, I think that's a good idea. I'll look to consolidate some of the code in the logs SDK first. Looking at the spec again, we should apply limits in scope attributes too.

[pichlermarc]

I refactored some of the code to so that we have a shared utility in the Logs SDK for handling limits and adding attributes. Now it also applies limits for scope-attributes.

I've not changed behavior yet how we count dropped attributes, though. Please let me know if we should change one of them (either scope attr handling, or log record attr handling) 🙂

Ref: 98b6f26

[trentm]

Assuming #6349 or similar adds ExtendedAttributes to the @opentelemetry/api, would we want to consider updating InstrumentationScope in @opentelemetry/core to support attributes, rather than adding a separate LogInstrumentationScope type?

[pichlermarc]

would we want to consider updating InstrumentationScope in @opentelemetry/core to support attributes, rather than adding a separate LogInstrumentationScope type?

Yes, that's what I was thinking - also the reason why LogInstrumentationScope is not public yet; I'd rather have the type definition duplicated right now so that we don't need to deprecate and remove it later.

I'd like to implement this also for metrics and traces in a similar way. I want to:

1. extend the instrumentationScope properties in signal SDKs like so: InstrumentationScope & { attributes?: ExtendedAttributes, droppedAttributesCount?: number }
2. update InstrumentationScope in @opentelemetry/core so that it also contains these, and remove the extensions in the signal SDKs again so that it's unified across the board.

One caveat that just came to mind: we can only use the ExtendedAttributes in @opentelemetry/core@3.0.0. That's because we'd have to raise the minimum supported API version to use the new type exported from the future @opentelemetry/api@1.10.0

[pichlermarc]

If we agree that this is what we want to do I'll create some issues so that we don't forget when we work on 3.x :)

[trentm]

If we agree that this is what we want to do

Yes.

One caveat that just came to mind: we can only use the ExtendedAttributes in @opentelemetry/core@3.0.0. That's because we'd have to raise the minimum supported API version to use the new type exported from the future @opentelemetry/api@1.10.0

I agree.

(Aside: This paragraph is just sharing, and not suggesting anything concrete for this PR. What constitutes a breaking change here is still twisting my mind. My gut tells me that it is wrong that @opentelemetry/core should need a major version bump to add runtime support for a wider attributes type. I'm fairly confident I understand the "because we are sharing interface ExtendedAttributes" issue. However, I feel there should be a solution here. I don't know if it is a copied interface ExtendedAttributes that lives in @opentelemetry/core, or a separate @opentelemetry/api-types types-only package that both API and SDK packages use, or what. I haven't taken the time to play around here.)

[pichlermarc]

However, I feel there should be a solution here. I don't know if it is a copied interface ExtendedAttributes that lives in @opentelemetry/core, or a separate @opentelemetry/api-types types-only package that both API and SDK packages use, or what. I haven't taken the time to play around here.)

Mhm, the approach of copying the type is what we did in the past, since it was the simplest at the time (we only had to copy it to one place). I think we can do the same here.