Skip to content

Commit

Permalink
fix: ensure default telemetry attributes are sent (#145)
Browse files Browse the repository at this point in the history
* fix: ensure default telemetry attributes are sent

* fix configuration, don't send non-configured metrics, tests

* create default config in Metrics

* null metrics indicate nothing sent
  • Loading branch information
jimmyjames authored Feb 18, 2025
1 parent de4e783 commit f487f6c
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 22 deletions.
3 changes: 2 additions & 1 deletion src/main/java/dev/openfga/sdk/api/auth/OAuth2Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public OAuth2Client(Configuration configuration, ApiClient apiClient) throws Fga
this.config = new Configuration()
.apiUrl(buildApiTokenIssuer(clientCredentials.getApiTokenIssuer()))
.maxRetries(configuration.getMaxRetries())
.minimumRetryDelay(configuration.getMinimumRetryDelay());
.minimumRetryDelay(configuration.getMinimumRetryDelay())
.telemetryConfiguration(configuration.getTelemetryConfiguration());
this.telemetry = new Telemetry(this.config);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,10 @@ public ClientConfiguration minimumRetryDelay(Duration minimumRetryDelay) {
super.minimumRetryDelay(minimumRetryDelay);
return this;
}

@Override
public ClientConfiguration telemetryConfiguration(TelemetryConfiguration telemetryConfiguration) {
super.telemetryConfiguration(telemetryConfiguration);
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,64 @@
import java.util.Map;
import java.util.Optional;

/**
* Configures the telemetry settings for the SDK.
*/
public class TelemetryConfiguration {
private Map<Metric, Map<Attribute, Optional<Object>>> metrics = new HashMap<>();

public TelemetryConfiguration() {
Map<Attribute, Optional<Object>> defaultAttributes = new HashMap<>();
defaultAttributes.put(Attributes.FGA_CLIENT_REQUEST_CLIENT_ID, Optional.empty());
defaultAttributes.put(Attributes.FGA_CLIENT_REQUEST_METHOD, Optional.empty());
defaultAttributes.put(Attributes.FGA_CLIENT_REQUEST_MODEL_ID, Optional.empty());
defaultAttributes.put(Attributes.FGA_CLIENT_REQUEST_STORE_ID, Optional.empty());
defaultAttributes.put(Attributes.FGA_CLIENT_RESPONSE_MODEL_ID, Optional.empty());
defaultAttributes.put(Attributes.HTTP_HOST, Optional.empty());
defaultAttributes.put(Attributes.HTTP_REQUEST_METHOD, Optional.empty());
defaultAttributes.put(Attributes.HTTP_REQUEST_RESEND_COUNT, Optional.empty());
defaultAttributes.put(Attributes.HTTP_RESPONSE_STATUS_CODE, Optional.empty());
defaultAttributes.put(Attributes.URL_FULL, Optional.empty());
defaultAttributes.put(Attributes.URL_SCHEME, Optional.empty());
defaultAttributes.put(Attributes.USER_AGENT, Optional.empty());
private static final Map<Attribute, Optional<Object>> defaultAttributes = Map.ofEntries(
Map.entry(Attributes.FGA_CLIENT_REQUEST_CLIENT_ID, Optional.empty()),
Map.entry(Attributes.FGA_CLIENT_REQUEST_METHOD, Optional.empty()),
Map.entry(Attributes.FGA_CLIENT_REQUEST_MODEL_ID, Optional.empty()),
Map.entry(Attributes.FGA_CLIENT_REQUEST_STORE_ID, Optional.empty()),
Map.entry(Attributes.FGA_CLIENT_RESPONSE_MODEL_ID, Optional.empty()),
Map.entry(Attributes.HTTP_HOST, Optional.empty()),
Map.entry(Attributes.HTTP_REQUEST_METHOD, Optional.empty()),
Map.entry(Attributes.HTTP_REQUEST_RESEND_COUNT, Optional.empty()),
Map.entry(Attributes.HTTP_RESPONSE_STATUS_CODE, Optional.empty()),
Map.entry(Attributes.URL_FULL, Optional.empty()),
Map.entry(Attributes.URL_SCHEME, Optional.empty()),
Map.entry(Attributes.USER_AGENT, Optional.empty()));

/**
* Constructs a TelemetryConfiguration object with the the metrics and attributes to send by default.
*/
public TelemetryConfiguration() {
metrics.put(Counters.CREDENTIALS_REQUEST, defaultAttributes);
metrics.put(Histograms.QUERY_DURATION, defaultAttributes);
metrics.put(Histograms.REQUEST_DURATION, defaultAttributes);
}

/**
* Constructs a TelemetryConfiguration object with the specified metrics.
* @param metrics the metrics to send
*/
public TelemetryConfiguration(Map<Metric, Map<Attribute, Optional<Object>>> metrics) {
this.metrics = metrics;
}

/**
* Sets the metrics to send.
* @param metrics the metrics to send
* @return this TelemetryConfiguration object
*/
public TelemetryConfiguration metrics(Map<Metric, Map<Attribute, Optional<Object>>> metrics) {
this.metrics = metrics;
return this;
}

/**
* @return the metrics to send.
*/
public Map<Metric, Map<Attribute, Optional<Object>>> metrics() {
return metrics;
}

/**
* @return the default attributes to send.
*/
public static Map<Attribute, Optional<Object>> defaultAttributes() {
return defaultAttributes;
}
}
18 changes: 17 additions & 1 deletion src/main/java/dev/openfga/sdk/telemetry/Metrics.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dev.openfga.sdk.telemetry;

import dev.openfga.sdk.api.configuration.Configuration;
import dev.openfga.sdk.api.configuration.TelemetryConfiguration;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.metrics.DoubleHistogram;
import io.opentelemetry.api.metrics.LongCounter;
Expand All @@ -23,13 +24,17 @@ public Metrics() {
this.counters = new HashMap<>();
this.histograms = new HashMap<>();
this.configuration = new Configuration();
this.configuration.telemetryConfiguration(new TelemetryConfiguration());
}

public Metrics(Configuration configuration) {
this.meter = OpenTelemetry.noop().getMeterProvider().get("openfga-sdk");
this.counters = new HashMap<>();
this.histograms = new HashMap<>();
this.configuration = configuration;
if (this.configuration.getTelemetryConfiguration() == null) {
this.configuration.telemetryConfiguration(new TelemetryConfiguration());
}
}

/**
Expand All @@ -48,9 +53,13 @@ public Meter getMeter() {
* @param value The value to be added to the counter.
* @param attributes A map of attributes associated with the metric.
*
* @return The LongCounter metric instance.
* @return The LongCounter metric instance, if the counter was configured. Otherwise, null.
*/
public LongCounter getCounter(Counter counter, Long value, Map<Attribute, String> attributes) {
if (configuration.getTelemetryConfiguration().metrics() == null
|| !configuration.getTelemetryConfiguration().metrics().containsKey(counter)) {
return null;
}
if (!counters.containsKey(counter.getName())) {
counters.put(
counter.getName(),
Expand All @@ -74,8 +83,15 @@ public LongCounter getCounter(Counter counter, Long value, Map<Attribute, String
* @param histogram The Histogram enum representing the metric.
* @param value The value to be recorded in the histogram.
* @param attributes A map of attributes associated with the metric.
*
* @return the DoubleHistogram instance, if the histogram was configured. Otherwise, null.
*/
public DoubleHistogram getHistogram(Histogram histogram, Double value, Map<Attribute, String> attributes) {
if (configuration.getTelemetryConfiguration().metrics() == null
|| !configuration.getTelemetryConfiguration().metrics().containsKey(histogram)) {
return null;
}

if (!histograms.containsKey(histogram.getName())) {
histograms.put(
histogram.getName(),
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/dev/openfga/sdk/telemetry/Telemetry.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* The Telemetry class provides access to telemetry-related functionality.
*/
public class Telemetry {
private Configuration configuration = new Configuration();
private Configuration configuration = null;
private Metrics metrics = null;

public Telemetry(Configuration configuration) {
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class OpenFgaApiTest {
private static final String EMPTY_RESPONSE_BODY = "{}";
private static final int DEFAULT_MAX_RETRIES = 3;
private static final Duration DEFAULT_RETRY_DELAY = Duration.ofMillis(100);
private static final TelemetryConfiguration DEFAULT_TELEMETRY_CONFIG = new TelemetryConfiguration();

private final ObjectMapper mapper = new ObjectMapper();
private OpenFgaApi fga;
Expand All @@ -72,6 +73,7 @@ public void beforeEachTest() throws Exception {
when(mockConfiguration.getCredentials()).thenReturn(new Credentials());
when(mockConfiguration.getMaxRetries()).thenReturn(DEFAULT_MAX_RETRIES);
when(mockConfiguration.getMinimumRetryDelay()).thenReturn(DEFAULT_RETRY_DELAY);
when(mockConfiguration.getTelemetryConfiguration()).thenReturn(DEFAULT_TELEMETRY_CONFIG);

mockApiClient = mock(ApiClient.class);
when(mockApiClient.getObjectMapper()).thenReturn(mapper);
Expand Down
46 changes: 41 additions & 5 deletions src/test/java/dev/openfga/sdk/telemetry/AttributesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,16 @@ void testPrepare() {
// Arrange
Map<Attribute, String> attributes = new HashMap<>();
attributes.put(Attributes.FGA_CLIENT_REQUEST_CLIENT_ID, "client-id-value");
Metric metric = mock(Metric.class);
Metric metric = Histograms.QUERY_DURATION;

TelemetryConfiguration telemetryConfiguration = mock(TelemetryConfiguration.class);
Map<Metric, Map<Attribute, Optional<Object>>> metricsMap = new HashMap<>();
Map<Attribute, Optional<Object>> attributeMap = new HashMap<>();
attributeMap.put(Attributes.FGA_CLIENT_REQUEST_CLIENT_ID, Optional.of("config-value"));
metricsMap.put(metric, attributeMap);
when(telemetryConfiguration.metrics()).thenReturn(metricsMap);
TelemetryConfiguration telemetryConfiguration = new TelemetryConfiguration(metricsMap);

Configuration configuration = mock(Configuration.class);
when(configuration.getTelemetryConfiguration()).thenReturn(telemetryConfiguration);
Configuration configuration = new Configuration();
configuration.telemetryConfiguration(telemetryConfiguration);

// Act
io.opentelemetry.api.common.Attributes result = Attributes.prepare(attributes, metric, configuration);
Expand All @@ -47,6 +46,43 @@ void testPrepare() {
assertEquals(expected, result);
}

@Test
void testPrepare_filtersAttributesFromDefaults() {
// Arrange

// sent by default
Map<Attribute, String> defaultAttributes = new HashMap<>();
for (Map.Entry<Attribute, Optional<Object>> entry :
TelemetryConfiguration.defaultAttributes().entrySet()) {
defaultAttributes.put(entry.getKey(), entry.getKey().toString() + "-value");
}

// not sent by default
Map<Attribute, String> nonDefaultAttributes = new HashMap<>();
nonDefaultAttributes.put(Attributes.FGA_CLIENT_USER, "user-value");

Map<Attribute, String> attributes = new HashMap<>();
attributes.putAll(defaultAttributes);
attributes.putAll(nonDefaultAttributes);

Metric metric = Histograms.QUERY_DURATION;

Configuration configuration = new Configuration();
configuration.telemetryConfiguration(new TelemetryConfiguration());

// Act
io.opentelemetry.api.common.Attributes result = Attributes.prepare(attributes, metric, configuration);

// Assert
AttributesBuilder builder = io.opentelemetry.api.common.Attributes.builder();
for (Map.Entry<Attribute, String> entry : defaultAttributes.entrySet()) {
builder.put(AttributeKey.stringKey(entry.getKey().getName()), entry.getValue());
}
io.opentelemetry.api.common.Attributes expected = builder.build();

assertEquals(expected, result);
}

@Test
void testFromHttpResponse() {
// Arrange
Expand Down
97 changes: 97 additions & 0 deletions src/test/java/dev/openfga/sdk/telemetry/MetricsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import dev.openfga.sdk.api.configuration.Configuration;
import dev.openfga.sdk.api.configuration.TelemetryConfiguration;
import io.opentelemetry.api.metrics.DoubleHistogram;
import io.opentelemetry.api.metrics.LongCounter;
import io.opentelemetry.api.metrics.Meter;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -21,6 +24,8 @@ class MetricsTest {
void setUp() {
configuration = mock(Configuration.class);
metrics = new Metrics(configuration);

when(configuration.getTelemetryConfiguration()).thenReturn(new TelemetryConfiguration());
}

@Test
Expand Down Expand Up @@ -116,4 +121,96 @@ void testQueryDuration() {
// Assert
assertNotNull(doubleHistogram, "The DoubleHistogram object should not be null.");
}

@Test
void testMetricsNotSentIfNotConfigured() {
Map<Attribute, Optional<Object>> attributes = new HashMap<>();
attributes.put(Attributes.FGA_CLIENT_REQUEST_METHOD, Optional.empty());
Map<Metric, Map<Attribute, Optional<Object>>> metrics = new HashMap<>();
metrics.put(Histograms.QUERY_DURATION, attributes);
TelemetryConfiguration telemetryConfiguration = new TelemetryConfiguration(metrics);

Configuration config = new Configuration();
config.telemetryConfiguration(telemetryConfiguration);

Metrics configuredMetrics = new Metrics(config);

DoubleHistogram requestDuration =
configuredMetrics.getHistogram(Histograms.REQUEST_DURATION, 10.0, new HashMap<>());
assertNull(requestDuration, "Unconfigured histograms should not be sent.");

DoubleHistogram queryDuration =
configuredMetrics.getHistogram(Histograms.QUERY_DURATION, 10.0, new HashMap<>());
assertNotNull(queryDuration, "Configured histograms should be sent.");

LongCounter credsRequestCounter =
configuredMetrics.getCounter(Counters.CREDENTIALS_REQUEST, 10L, new HashMap<>());
assertNull(credsRequestCounter, "Unconfigured counters should not be sent.");
}

@Test
void testCountersNotSentIfNotConfigured() {
Map<Attribute, Optional<Object>> attributes = new HashMap<>();
attributes.put(Attributes.FGA_CLIENT_REQUEST_METHOD, Optional.empty());
Map<Metric, Map<Attribute, Optional<Object>>> metrics = new HashMap<>();
metrics.put(Counters.CREDENTIALS_REQUEST, attributes);
TelemetryConfiguration telemetryConfiguration = new TelemetryConfiguration(metrics);

Configuration config = new Configuration();
config.telemetryConfiguration(telemetryConfiguration);

Metrics configuredMetrics = new Metrics(config);

DoubleHistogram requestDuration =
configuredMetrics.getHistogram(Histograms.REQUEST_DURATION, 10.0, new HashMap<>());
assertNull(requestDuration, "Unconfigured histograms should not be sent.");

DoubleHistogram queryDuration =
configuredMetrics.getHistogram(Histograms.QUERY_DURATION, 10.0, new HashMap<>());
assertNull(queryDuration, "Unconfigured histograms should not be sent.");

LongCounter credsCounter = configuredMetrics.getCounter(Counters.CREDENTIALS_REQUEST, 10L, new HashMap<>());
assertNotNull(credsCounter, "Configured counter should be sent.");
}

@Test
void testDefaultMetricsEnabled() {
// Arrange
Configuration config = new Configuration();

// Act
Metrics metrics = new Metrics(config);

// Assert
assertNotNull(
metrics.getCounter(Counters.CREDENTIALS_REQUEST, 10L, new HashMap<>()), "The counter should be sent.");
assertNotNull(
metrics.getHistogram(Histograms.QUERY_DURATION, 10.0, new HashMap<>()),
"The query duration histogram should be sent.");
assertNotNull(
metrics.getHistogram(Histograms.REQUEST_DURATION, 10.0, new HashMap<>()),
"The request duration histogram should be sent.");
}

@Test
void testMetricsWithNullMetricsConfig() {
// Arrange
TelemetryConfiguration telemetryConfiguration = new TelemetryConfiguration(null);
Configuration config = new Configuration();
config.telemetryConfiguration(telemetryConfiguration);

// Act
Metrics metrics = new Metrics(config);

// Assert
assertNull(
metrics.getCounter(Counters.CREDENTIALS_REQUEST, 10L, new HashMap<>()),
"The counter should not be sent.");
assertNull(
metrics.getHistogram(Histograms.QUERY_DURATION, 10.0, new HashMap<>()),
"The query duration histogram should not be sent.");
assertNull(
metrics.getHistogram(Histograms.REQUEST_DURATION, 10.0, new HashMap<>()),
"The request duration histogram should not be sent.");
}
}

0 comments on commit f487f6c

Please sign in to comment.