From 3b5eb239cc2612d2181f4661a5be6f60552026b3 Mon Sep 17 00:00:00 2001
From: Hyun Oh <hyunnoh01@gmail.com>
Date: Mon, 15 Apr 2024 20:13:45 +0900
Subject: [PATCH] feat(sdk-logs): make dropping attribute print message (#4614)

* feat(sdk-logs): make dropping attribute print message

* chore: update changelog

* chore(sdk-logs): add comment to explain message logic

* Update experimental/CHANGELOG.md

---------

Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
---
 experimental/CHANGELOG.md                     |  1 +
 .../packages/sdk-logs/src/LogRecord.ts        |  4 ++
 .../sdk-logs/test/common/LogRecord.test.ts    | 67 ++++++++++++-------
 3 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
index c2ad7bcaed9..c94f75f1ad3 100644
--- a/experimental/CHANGELOG.md
+++ b/experimental/CHANGELOG.md
@@ -16,6 +16,7 @@ All notable changes to experimental packages in this project will be documented
 ### :rocket: (Enhancement)
 
 * feat(otlp-transformer): consolidate scope/resource creation in transformer [#4600](https://github.com/open-telemetry/opentelemetry-js/pull/4600)
+* feat(sdk-logs): print message when attributes are dropped due to attribute count limit [#4614](https://github.com/open-telemetry/opentelemetry-js/pull/4614) @HyunnoH
 
 ### :bug: (Bug Fix)
 
diff --git a/experimental/packages/sdk-logs/src/LogRecord.ts b/experimental/packages/sdk-logs/src/LogRecord.ts
index abc4f452855..e7704ec3892 100644
--- a/experimental/packages/sdk-logs/src/LogRecord.ts
+++ b/experimental/packages/sdk-logs/src/LogRecord.ts
@@ -140,6 +140,10 @@ export class LogRecord implements ReadableLogRecord {
         this._logRecordLimits.attributeCountLimit &&
       !Object.prototype.hasOwnProperty.call(this.attributes, key)
     ) {
+      // This logic is to create drop message at most once per LogRecord to prevent excessive logging.
+      if (this.droppedAttributesCount === 1) {
+        api.diag.warn('Dropping extra attributes.');
+      }
       return this;
     }
     if (isAttributeValue(value)) {
diff --git a/experimental/packages/sdk-logs/test/common/LogRecord.test.ts b/experimental/packages/sdk-logs/test/common/LogRecord.test.ts
index 5a0acaa6db2..3518e99fd18 100644
--- a/experimental/packages/sdk-logs/test/common/LogRecord.test.ts
+++ b/experimental/packages/sdk-logs/test/common/LogRecord.test.ts
@@ -177,32 +177,31 @@ describe('LogRecord', () => {
 
     describe('when logRecordLimits options set', () => {
       describe('when "attributeCountLimit" option defined', () => {
-        const { logRecord } = setup({ attributeCountLimit: 100 });
-        for (let i = 0; i < 150; i++) {
-          let attributeValue;
-          switch (i % 3) {
-            case 0: {
-              attributeValue = `bar${i}`;
-              break;
-            }
-            case 1: {
-              attributeValue = [`bar${i}`];
-              break;
-            }
-            case 2: {
-              attributeValue = {
-                bar: `bar${i}`,
-              };
-              break;
-            }
-            default: {
-              attributeValue = `bar${i}`;
+        it('should remove / drop all remaining values after the number of values exceeds this limit', () => {
+          const { logRecord } = setup({ attributeCountLimit: 100 });
+          for (let i = 0; i < 150; i++) {
+            let attributeValue;
+            switch (i % 3) {
+              case 0: {
+                attributeValue = `bar${i}`;
+                break;
+              }
+              case 1: {
+                attributeValue = [`bar${i}`];
+                break;
+              }
+              case 2: {
+                attributeValue = {
+                  bar: `bar${i}`,
+                };
+                break;
+              }
+              default: {
+                attributeValue = `bar${i}`;
+              }
             }
+            logRecord.setAttribute(`foo${i}`, attributeValue);
           }
-          logRecord.setAttribute(`foo${i}`, attributeValue);
-        }
-
-        it('should remove / drop all remaining values after the number of values exceeds this limit', () => {
           const { attributes, droppedAttributesCount } = logRecord;
           assert.strictEqual(Object.keys(attributes).length, 100);
           assert.strictEqual(attributes.foo0, 'bar0');
@@ -212,6 +211,26 @@ describe('LogRecord', () => {
           assert.strictEqual(attributes.foo149, undefined);
           assert.strictEqual(droppedAttributesCount, 50);
         });
+
+        it('should not print message when there are no dropped attributes', () => {
+          const warnStub = sinon.spy(diag, 'warn');
+          const { logRecord } = setup({ attributeCountLimit: 10 });
+          for (let i = 0; i < 7; i++) {
+            logRecord.setAttribute(`foo${i}`, `bar${i}`);
+          }
+          sinon.assert.callCount(warnStub, 0);
+          warnStub.restore();
+        });
+
+        it('should print message only once when attribute(s) are dropped', () => {
+          const warnStub = sinon.spy(diag, 'warn');
+          const { logRecord } = setup({ attributeCountLimit: 5 });
+          for (let i = 0; i < 7; i++) {
+            logRecord.setAttribute(`foo${i}`, `bar${i}`);
+          }
+          sinon.assert.callCount(warnStub, 1);
+          warnStub.restore();
+        });
       });
 
       describe('when "attributeValueLengthLimit" option defined', () => {