Skip to content

Commit 95c6fb9

Browse files
committed
Address internal review feedback
- Remove exporterIsConfiguredWithCumulativeTemporalityForCounters test (tested OTel SDK, not our code; the integration test is the real regression guard) - Fix Provider catch block comment to reflect that FlagEvalMetrics may not have logged if we reach this point - Include exception in log.error calls for NoClassDefFoundError and general Exception to aid debugging - Reword InMemoryMetricReader comment for precision
1 parent c3a7955 commit 95c6fb9

3 files changed

Lines changed: 7 additions & 24 deletions

File tree

products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/FlagEvalMetrics.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,12 @@ class FlagEvalMetrics implements Closeable {
8585
log.error(
8686
"Evaluation logging is enabled but OpenTelemetry SDK is not on the classpath. "
8787
+ "Add opentelemetry-sdk-metrics and opentelemetry-exporter-otlp to your dependencies, "
88-
+ "or disable evaluation logging via Provider.Options.evaluationLogging(false).");
88+
+ "or disable evaluation logging via Provider.Options.evaluationLogging(false).",
89+
e);
8990
counter = null;
9091
meterProvider = null;
9192
} catch (Exception e) {
92-
log.error("Failed to initialize flag evaluation metrics: {}", e.getMessage());
93+
log.error("Failed to initialize flag evaluation metrics", e);
9394
counter = null;
9495
meterProvider = null;
9596
}

products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public Provider(final Options options) {
4949
metrics = new FlagEvalMetrics();
5050
hook = new FlagEvalHook(metrics);
5151
} catch (NoClassDefFoundError | Exception e) {
52-
// FlagEvalMetrics logs the error — degrade to no-op
52+
// Fallback: FlagEvalMetrics constructor threw unexpectedly — degrade to no-op
5353
}
5454
}
5555
this.flagEvalMetrics = metrics;

products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/FlagEvalMetricsTest.java

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,10 @@
99
import dev.openfeature.sdk.ErrorCode;
1010
import io.opentelemetry.api.common.Attributes;
1111
import io.opentelemetry.api.metrics.LongCounter;
12-
import io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporter;
13-
import io.opentelemetry.sdk.metrics.InstrumentType;
1412
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
1513
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
1614
import io.opentelemetry.sdk.metrics.data.LongPointData;
1715
import io.opentelemetry.sdk.metrics.data.MetricData;
18-
import io.opentelemetry.sdk.metrics.export.AggregationTemporalitySelector;
1916
import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader;
2017
import java.util.Collection;
2118
import org.junit.jupiter.api.Test;
@@ -147,26 +144,11 @@ void shutdownClearsCounter() {
147144
verifyNoInteractions(counter);
148145
}
149146

150-
@Test
151-
void exporterIsConfiguredWithCumulativeTemporalityForCounters() {
152-
// Regression guard: FlagEvalMetrics must explicitly configure alwaysCumulative() so that
153-
// the Datadog agent receives absolute counts rather than delta values that may be converted
154-
// to rates. This test documents and enforces that the exporter uses CUMULATIVE for counters.
155-
try (OtlpHttpMetricExporter exporter =
156-
OtlpHttpMetricExporter.builder()
157-
.setAggregationTemporalitySelector(AggregationTemporalitySelector.alwaysCumulative())
158-
.build()) {
159-
assertEquals(
160-
AggregationTemporality.CUMULATIVE,
161-
exporter.getAggregationTemporality(InstrumentType.COUNTER),
162-
"alwaysCumulative() selector must produce CUMULATIVE for counters");
163-
}
164-
}
165-
166147
@Test
167148
void multipleRecordCallsAccumulateCumulativelyInExportedMetrics() {
168-
// Use InMemoryMetricReader with cumulative temporality (matching what FlagEvalMetrics
169-
// configures on the OTLP exporter) to verify that N record() calls produce a sum of N.
149+
// InMemoryMetricReader defaults to cumulative temporality. This validates that N record()
150+
// calls produce a cumulative sum of N, matching the alwaysCumulative() selector configured
151+
// on the production OTLP exporter in FlagEvalMetrics.
170152
InMemoryMetricReader reader = InMemoryMetricReader.create();
171153
SdkMeterProvider provider = SdkMeterProvider.builder().registerMetricReader(reader).build();
172154

0 commit comments

Comments
 (0)