From a50ceb3959ec6d0f3b184bb74790c6c0dc3d7ac3 Mon Sep 17 00:00:00 2001 From: jack-berg <34418638+jack-berg@users.noreply.github.com> Date: Fri, 16 Sep 2022 10:49:06 -0500 Subject: [PATCH] Implement dropped attribute count in logs (#4697) * Implement dropped attribute count in logs * PR feedback * PR feedback --- .../internal/otlp/logs/LogMarshaler.java | 3 +-- .../otlp/logs/LogsRequestMarshalerTest.java | 4 ++++ .../sdk/testing/assertj/LogDataAssert.java | 13 +++++++++++ .../sdk/testing/logs/TestLogData.java | 6 ++++- .../testing/assertj/LogAssertionsTest.java | 6 ++++- .../io/opentelemetry/sdk/logs/SdkLogData.java | 6 +++-- .../sdk/logs/SdkReadWriteLogRecord.java | 3 ++- .../opentelemetry/sdk/logs/data/LogData.java | 10 ++++++++ .../sdk/logs/SdkLogEmitterTest.java | 23 ++++++++++--------- 9 files changed, 56 insertions(+), 18 deletions(-) diff --git a/exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/logs/LogMarshaler.java b/exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/logs/LogMarshaler.java index f5fa3d60b3..23b0855805 100644 --- a/exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/logs/LogMarshaler.java +++ b/exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/logs/LogMarshaler.java @@ -50,8 +50,7 @@ final class LogMarshaler extends MarshalerWithSize { MarshalerUtil.toBytes(logData.getSeverityText()), anyValueMarshaler, attributeMarshalers, - // TODO (trask) implement droppedAttributesCount in LogRecord - 0, + logData.getTotalAttributeCount() - logData.getAttributes().size(), spanContext.getTraceFlags(), spanContext.getTraceId().equals(INVALID_TRACE_ID) ? null : spanContext.getTraceId(), spanContext.getSpanId().equals(INVALID_SPAN_ID) ? null : spanContext.getSpanId()); diff --git a/exporters/otlp/common/src/test/java/io/opentelemetry/exporter/internal/otlp/logs/LogsRequestMarshalerTest.java b/exporters/otlp/common/src/test/java/io/opentelemetry/exporter/internal/otlp/logs/LogsRequestMarshalerTest.java index 98886745be..e8ecde905c 100644 --- a/exporters/otlp/common/src/test/java/io/opentelemetry/exporter/internal/otlp/logs/LogsRequestMarshalerTest.java +++ b/exporters/otlp/common/src/test/java/io/opentelemetry/exporter/internal/otlp/logs/LogsRequestMarshalerTest.java @@ -69,6 +69,7 @@ class LogsRequestMarshalerTest { SpanContext.create( TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())) .setAttributes(Attributes.of(AttributeKey.booleanKey("key"), true)) + .setTotalAttributeCount(2) .setEpoch(12345, TimeUnit.NANOSECONDS) .build())); @@ -111,6 +112,7 @@ class LogsRequestMarshalerTest { SpanContext.create( TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())) .setAttributes(Attributes.of(AttributeKey.booleanKey("key"), true)) + .setTotalAttributeCount(2) .setEpoch(12345, TimeUnit.NANOSECONDS) .build())); @@ -124,6 +126,7 @@ class LogsRequestMarshalerTest { .setKey("key") .setValue(AnyValue.newBuilder().setBoolValue(true).build()) .build()); + assertThat(logRecord.getDroppedAttributesCount()).isEqualTo(1); assertThat(logRecord.getTimeUnixNano()).isEqualTo(12345); } @@ -148,6 +151,7 @@ class LogsRequestMarshalerTest { .isEqualTo(Severity.UNDEFINED_SEVERITY_NUMBER.getSeverityNumber()); assertThat(logRecord.getBody()).isEqualTo(AnyValue.newBuilder().setStringValue("").build()); assertThat(logRecord.getAttributesList()).isEmpty(); + assertThat(logRecord.getDroppedAttributesCount()).isZero(); assertThat(logRecord.getTimeUnixNano()).isEqualTo(12345); } diff --git a/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LogDataAssert.java b/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LogDataAssert.java index c2e853a8db..e77d23675f 100644 --- a/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LogDataAssert.java +++ b/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LogDataAssert.java @@ -163,4 +163,17 @@ public class LogDataAssert extends AbstractAssert { // implementations. return actual.getAttributes().asMap().equals(attributes.asMap()); } + + /** Asserts the log has the given total attributes. */ + public LogDataAssert hasTotalAttributeCount(int totalAttributeCount) { + isNotNull(); + if (actual.getTotalAttributeCount() != totalAttributeCount) { + failWithActualExpectedAndMessage( + actual.getTotalAttributeCount(), + totalAttributeCount, + "Expected log to have recorded <%s> total attributes but did not", + totalAttributeCount); + } + return this; + } } diff --git a/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/logs/TestLogData.java b/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/logs/TestLogData.java index 4b6f7f9305..00b82f0020 100644 --- a/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/logs/TestLogData.java +++ b/sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/logs/TestLogData.java @@ -31,7 +31,8 @@ public abstract class TestLogData implements LogData { .setSpanContext(SpanContext.getInvalid()) .setSeverity(Severity.UNDEFINED_SEVERITY_NUMBER) .setBody("") - .setAttributes(Attributes.empty()); + .setAttributes(Attributes.empty()) + .setTotalAttributeCount(0); } TestLogData() {} @@ -86,5 +87,8 @@ public abstract class TestLogData implements LogData { /** Set the attributes. */ public abstract Builder setAttributes(Attributes attributes); + + /** Set the total attribute count. */ + public abstract Builder setTotalAttributeCount(int totalAttributeCount); } } diff --git a/sdk/logs-testing/src/test/java/io/opentelemetry/sdk/testing/assertj/LogAssertionsTest.java b/sdk/logs-testing/src/test/java/io/opentelemetry/sdk/testing/assertj/LogAssertionsTest.java index ab228163b6..1941285676 100644 --- a/sdk/logs-testing/src/test/java/io/opentelemetry/sdk/testing/assertj/LogAssertionsTest.java +++ b/sdk/logs-testing/src/test/java/io/opentelemetry/sdk/testing/assertj/LogAssertionsTest.java @@ -55,6 +55,7 @@ public class LogAssertionsTest { .setSeverityText("info") .setBody("message") .setAttributes(ATTRIBUTES) + .setTotalAttributeCount(999) .build(); @Test @@ -109,7 +110,8 @@ public class LogAssertionsTest { attributeEntry("colors", "red", "blue"), attributeEntry("conditions", false, true), attributeEntry("scores", 0L, 1L), - attributeEntry("coins", 0.01, 0.05, 0.1))); + attributeEntry("coins", 0.01, 0.05, 0.1))) + .hasTotalAttributeCount(999); } @Test @@ -179,5 +181,7 @@ public class LogAssertionsTest { AttributeKey.stringKey("bear"), value -> assertThat(value).hasSize(2)))) .isInstanceOf(AssertionError.class); + assertThatThrownBy(() -> assertThat(LOG_DATA).hasTotalAttributeCount(11)) + .isInstanceOf(AssertionError.class); } } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogData.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogData.java index 1a470a68a1..f488a33b75 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogData.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogData.java @@ -31,7 +31,8 @@ abstract class SdkLogData implements LogData { Severity severity, @Nullable String severityText, Body body, - Attributes attributes) { + Attributes attributes, + int totalAttributeCount) { return new AutoValue_SdkLogData( resource, instrumentationScopeInfo, @@ -40,6 +41,7 @@ abstract class SdkLogData implements LogData { severity, severityText, body, - attributes); + attributes, + totalAttributeCount); } } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java index bb1eb6336d..de9560bff4 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java @@ -115,7 +115,8 @@ class SdkReadWriteLogRecord implements ReadWriteLogRecord { severity, severityText, body, - getImmutableAttributes()); + getImmutableAttributes(), + attributes == null ? 0 : attributes.getTotalAddedValues()); } } } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/data/LogData.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/data/LogData.java index db207d3f1c..216ff3edd0 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/data/LogData.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/data/LogData.java @@ -8,6 +8,7 @@ package io.opentelemetry.sdk.logs.data; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.logs.LogLimits; import io.opentelemetry.sdk.resources.Resource; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -44,4 +45,13 @@ public interface LogData { /** Returns the attributes for this log, or {@link Attributes#empty()} if unset. */ Attributes getAttributes(); + + /** + * Returns the total number of attributes that were recorded on this log. + * + *

This number may be larger than the number of attributes that are attached to this log, if + * the total number recorded was greater than the configured maximum value. See {@link + * LogLimits#getMaxNumberOfAttributes()}. + */ + int getTotalAttributeCount(); } diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLogEmitterTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLogEmitterTest.java index 03ee9a43ec..0cc86b91f0 100644 --- a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLogEmitterTest.java +++ b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLogEmitterTest.java @@ -17,6 +17,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.internal.StringUtils; @@ -104,20 +105,20 @@ class SdkLogEmitterTest { () -> LogLimits.builder().setMaxNumberOfAttributes(maxNumberOfAttrs).build()) .build(); - AttributesBuilder attributesBuilder = Attributes.builder(); + LogRecordBuilder builder = logEmitterProvider.get("test").logRecordBuilder(); + AttributesBuilder expectedAttributes = Attributes.builder(); for (int i = 0; i < 2 * maxNumberOfAttrs; i++) { - attributesBuilder.put("key" + i, i); + AttributeKey key = AttributeKey.longKey("key" + i); + builder.setAttribute(key, (long) i); + if (i < maxNumberOfAttrs) { + expectedAttributes.put(key, (long) i); + } } + builder.emit(); - logEmitterProvider - .get("test") - .logRecordBuilder() - .setAllAttributes(attributesBuilder.build()) - .emit(); - - // NOTE: cannot guarantee which attributes are retained, only that there are no more that the - // max - assertThat(seenLog.get().toLogData().getAttributes()).hasSize(maxNumberOfAttrs); + assertThat(seenLog.get().toLogData()) + .hasAttributes(expectedAttributes.build()) + .hasTotalAttributeCount(maxNumberOfAttrs * 2); } @Test