fix(span,sdk-logs): enforce event/link attribute limits, link count limit, and fix LogRecord droppedAttributesCount

CHANGELOG.md

@@ -18,6 +18,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 
 * fix(opentelemetry-instrumentation): improve `_warnOnPreloadedModules` function not to show warning logs when the module is not marked as loaded [#6095](https://github.com/open-telemetry/opentelemetry-js/pull/6095) @rlj1202
 * fix(sdk-trace-base): derive internal `SpanOptions` from API type to prevent drift [#6478](https://github.com/open-telemetry/opentelemetry-js/pull/6478) @overbalance
+* fix(span): enforce `attributePerEventCountLimit`, `attributePerLinkCountLimit`, `linkCountLimit`, and `attributeValueLengthLimit` for event/link attributes [#6479](https://github.com/open-telemetry/opentelemetry-js/pull/6479) @overbalance
 
 ### :books: Documentation
 

experimental/CHANGELOG.md

@@ -17,6 +17,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 ### :bug: Bug Fixes
 
 * fix(opentelemetry-instrumentation): access `require` via `globalThis` to avoid webpack analysis [#6481](https://github.com/open-telemetry/opentelemetry-js/pull/6481) @overbalance
+* fix(sdk-logs): fix inflated `droppedAttributesCount` when updating existing attribute keys [#6479](https://github.com/open-telemetry/opentelemetry-js/pull/6479) @overbalance
 
 ### :books: Documentation
 

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

@@ -35,7 +35,8 @@ export class LogRecordImpl implements ReadableLogRecord {
   private _severityNumber?: SeverityNumber;
   private _body?: LogBody;
   private _eventName?: string;
-  private totalAttributesCount: number = 0;
+  private _attributesCount: number = 0;
+  private _droppedAttributesCount: number = 0;
 
   private _isReadonly: boolean = false;
   private readonly _logRecordLimits: Required<LogRecordLimits>;
@@ -81,7 +82,7 @@ export class LogRecordImpl implements ReadableLogRecord {
   }
 
   get droppedAttributesCount(): number {
-    return this.totalAttributesCount - Object.keys(this.attributes).length;
+    return this._droppedAttributesCount;
   }
 
   constructor(
@@ -136,19 +137,25 @@ export class LogRecordImpl implements ReadableLogRecord {
       api.diag.warn(`Invalid attribute value set for key: ${key}`);
       return this;
     }
-    this.totalAttributesCount += 1;
+    const isNewKey = !Object.prototype.hasOwnProperty.call(
+      this.attributes,
+      key
+    );
     if (
-      Object.keys(this.attributes).length >=
-        this._logRecordLimits.attributeCountLimit &&
-      !Object.prototype.hasOwnProperty.call(this.attributes, key)
+      isNewKey &&
+      this._attributesCount >= this._logRecordLimits.attributeCountLimit
     ) {
-      // This logic is to create drop message at most once per LogRecord to prevent excessive logging.
-      if (this.droppedAttributesCount === 1) {
+      this._droppedAttributesCount++;
+      // Only warn once per LogRecord to avoid log spam
+      if (this._droppedAttributesCount === 1) {
         api.diag.warn('Dropping extra attributes.');
       }
       return this;
     }
     this.attributes[key] = this._truncateToSize(value);
+    if (isNewKey) {
+      this._attributesCount++;
+    }
     return this;
   }
 

experimental/packages/sdk-logs/test/common/LogRecord.test.ts

@@ -301,6 +301,27 @@ describe('LogRecord', () => {
           sinon.assert.callCount(warnStub, 1);
           warnStub.restore();
         });
+
+        it('should not increment droppedAttributesCount on key updates', () => {
+          const { logRecord } = setup({ attributeCountLimit: 2 });
+          logRecord.setAttribute('key1', 'value1');
+          logRecord.setAttribute('key2', 'value2');
+          logRecord.setAttribute('key1', 'updated');
+          assert.strictEqual(logRecord.droppedAttributesCount, 0);
+          assert.strictEqual(logRecord.attributes['key1'], 'updated');
+        });
+
+        it('should correctly track droppedAttributesCount with key updates and dropped keys', () => {
+          const { logRecord } = setup({ attributeCountLimit: 2 });
+          logRecord.setAttribute('key1', 'value1');
+          logRecord.setAttribute('key2', 'value2');
+          logRecord.setAttribute('key3', 'value3');
+          logRecord.setAttribute('key1', 'updated');
+          assert.strictEqual(logRecord.droppedAttributesCount, 1);
+          assert.strictEqual(logRecord.attributes['key1'], 'updated');
+          assert.strictEqual(logRecord.attributes['key2'], 'value2');
+          assert.strictEqual(logRecord.attributes['key3'], undefined);
+        });
       });
 
       describe('when "attributeValueLengthLimit" option defined', () => {

packages/opentelemetry-sdk-trace-base/src/Span.ts

@@ -115,13 +115,17 @@ export class SpanImpl implements Span {
     this._startTimeProvided = opts.startTime != null;
     this._spanLimits = opts.spanLimits;
     this._attributeValueLengthLimit =
-      this._spanLimits.attributeValueLengthLimit || 0;
+      this._spanLimits.attributeValueLengthLimit ?? 0;
     this._spanProcessor = opts.spanProcessor;
 
     this.name = opts.name;
     this.parentSpanContext = opts.parentSpanContext;
     this.kind = opts.kind;
-    this.links = opts.links || [];
+    if (opts.links) {
+      for (const link of opts.links) {
+        this.addLink(link);
+      }
+    }
     this.startTime = this._getTime(opts.startTime ?? now);
     this.resource = opts.resource;
     this.instrumentationScope = opts.scope;
@@ -219,24 +223,83 @@ export class SpanImpl implements Span {
       attributesOrStartTime = undefined;
     }
 
-    const attributes = sanitizeAttributes(attributesOrStartTime);
+    const sanitized = sanitizeAttributes(attributesOrStartTime);
+    const { attributePerEventCountLimit } = this._spanLimits;
+    const entries = Object.entries(sanitized);
+    const attributes: Attributes = {};
+    let droppedAttributesCount = 0;
+
+    for (const [attr, attrVal] of entries) {
+      if (
+        attributePerEventCountLimit !== undefined &&
+        Object.keys(attributes).length >= attributePerEventCountLimit
+      ) {
+        droppedAttributesCount++;
+        continue;
+      }
+      attributes[attr] = this._truncateToSize(attrVal!);
+    }
 
     this.events.push({
       name,
       attributes,
       time: this._getTime(timeStamp),
-      droppedAttributesCount: 0,
+      droppedAttributesCount,
     });
     return this;
   }
 
   addLink(link: Link): this {
-    this.links.push(link);
+    if (this._isSpanEnded()) return this;
+
+    const { linkCountLimit } = this._spanLimits;
+
+    if (linkCountLimit === 0) {
+      this._droppedLinksCount++;
+      return this;
+    }
+
+    if (linkCountLimit !== undefined && this.links.length >= linkCountLimit) {
+      if (this._droppedLinksCount === 0) {
+        diag.debug('Dropping extra links.');
+      }
+      this.links.shift();
+      this._droppedLinksCount++;
+    }
+
+    const { attributePerLinkCountLimit } = this._spanLimits;
+    const sanitized = sanitizeAttributes(link.attributes);
+    const entries = Object.entries(sanitized);
+    const attributes: Attributes = {};
+    let droppedAttributesCount = 0;
+
+    for (const [attr, attrVal] of entries) {
+      if (
+        attributePerLinkCountLimit !== undefined &&
+        Object.keys(attributes).length >= attributePerLinkCountLimit
+      ) {
+        droppedAttributesCount++;
+        continue;
+      }
+      attributes[attr] = this._truncateToSize(attrVal!);
+    }
+
+    const processedLink: Link = { context: link.context };
+    if (Object.keys(attributes).length > 0) {
+      processedLink.attributes = attributes;
+    }
+    if (droppedAttributesCount > 0) {
+      processedLink.droppedAttributesCount = droppedAttributesCount;
+    }
+
+    this.links.push(processedLink);
     return this;
   }
 
   addLinks(links: Link[]): this {
-    this.links.push(...links);
+    for (const link of links) {
+      this.addLink(link);
+    }
     return this;
   }
 
@@ -296,6 +359,11 @@ export class SpanImpl implements Span {
         `Dropped ${this._droppedEventsCount} events because eventCountLimit reached`
       );
     }
+    if (this._droppedLinksCount > 0) {
+      diag.warn(
+        `Dropped ${this._droppedLinksCount} links because linkCountLimit reached`
+      );
+    }
     if (this._spanProcessor.onEnding) {
       this._spanProcessor.onEnding(this);
     }