Skip to content

Commit

Permalink
Add config option to disable attaching InstrumentationLibrary labels (#…
Browse files Browse the repository at this point in the history
…358)

* Add config option to disable InstrumentationLibraryLabels

* Showcase the config option via example

* Update exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java

Fix typo

Co-authored-by: David Ashpole <[email protected]>

---------

Co-authored-by: David Ashpole <[email protected]>
  • Loading branch information
psx95 and dashpole authored Jul 10, 2024
1 parent 7ca8f25 commit cfbc409
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ private static MetricConfiguration generateMetricExporterConfig(boolean useDefau
// Any properties not set would be retrieved from the default configuration of the exporter.
return MetricConfiguration.builder()
.setMetricServiceSettings(metricServiceSettingsBuilder.build())
.setInstrumentationLibraryLabelsEnabled(false)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,15 @@ public final class AggregateByLabelMetricTimeSeriesBuilder implements MetricTime
private final String prefix;
private final Predicate<AttributeKey<?>> resourceAttributeFilter;
private final MonitoredResourceDescription monitoredResourceDescription;
private final boolean instrumentationLibraryLabelsEnabled;

@Deprecated
public AggregateByLabelMetricTimeSeriesBuilder(String projectId, String prefix) {
this.projectId = projectId;
this.prefix = prefix;
this.resourceAttributeFilter = MetricConfiguration.NO_RESOURCE_ATTRIBUTES;
this.monitoredResourceDescription = MetricConfiguration.EMPTY_MONITORED_RESOURCE_DESCRIPTION;
this.instrumentationLibraryLabelsEnabled = true;
}

@Deprecated
Expand All @@ -76,8 +78,10 @@ public AggregateByLabelMetricTimeSeriesBuilder(
this.prefix = prefix;
this.resourceAttributeFilter = resourceAttributeFilter;
this.monitoredResourceDescription = MetricConfiguration.EMPTY_MONITORED_RESOURCE_DESCRIPTION;
this.instrumentationLibraryLabelsEnabled = true;
}

@Deprecated
public AggregateByLabelMetricTimeSeriesBuilder(
String projectId,
String prefix,
Expand All @@ -87,6 +91,20 @@ public AggregateByLabelMetricTimeSeriesBuilder(
this.prefix = prefix;
this.resourceAttributeFilter = resourceAttributeFilter;
this.monitoredResourceDescription = monitoredResourceDescription;
this.instrumentationLibraryLabelsEnabled = true;
}

public AggregateByLabelMetricTimeSeriesBuilder(
String projectId,
String prefix,
Predicate<AttributeKey<?>> resourceAttributeFilter,
MonitoredResourceDescription monitoredResourceDescription,
boolean instrumentationLibraryLabelsEnabled) {
this.projectId = projectId;
this.prefix = prefix;
this.resourceAttributeFilter = resourceAttributeFilter;
this.monitoredResourceDescription = monitoredResourceDescription;
this.instrumentationLibraryLabelsEnabled = instrumentationLibraryLabelsEnabled;
}

@Override
Expand Down Expand Up @@ -161,6 +179,9 @@ private Attributes extraLabelsFromResource(Resource resource) {

private Attributes instrumentationLibraryLabels(
Attributes attributes, InstrumentationScopeInfo instrumentationScopeInfo) {
if (!instrumentationLibraryLabelsEnabled) {
return attributes;
}
return attributes.toBuilder()
.put(
AttributeKey.stringKey(LABEL_INSTRUMENTATION_SOURCE),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class InternalMetricExporter implements MetricExporter {
private final Predicate<AttributeKey<?>> resourceAttributesFilter;
private final boolean useCreateServiceTimeSeries;
private final MonitoredResourceDescription monitoredResourceDescription;
private final boolean instrumentationLibraryLabelsEnabled;

InternalMetricExporter(
String projectId,
Expand All @@ -77,14 +78,16 @@ class InternalMetricExporter implements MetricExporter {
MetricDescriptorStrategy descriptorStrategy,
Predicate<AttributeKey<?>> resourceAttributesFilter,
boolean useCreateServiceTimeSeries,
MonitoredResourceDescription monitoredResourceDescription) {
MonitoredResourceDescription monitoredResourceDescription,
boolean instrumentationLibraryLabelsEnabled) {
this.projectId = projectId;
this.prefix = prefix;
this.metricServiceClient = client;
this.metricDescriptorStrategy = descriptorStrategy;
this.resourceAttributesFilter = resourceAttributesFilter;
this.useCreateServiceTimeSeries = useCreateServiceTimeSeries;
this.monitoredResourceDescription = monitoredResourceDescription;
this.instrumentationLibraryLabelsEnabled = instrumentationLibraryLabelsEnabled;
}

static InternalMetricExporter createWithConfiguration(MetricConfiguration configuration)
Expand All @@ -103,7 +106,8 @@ static InternalMetricExporter createWithConfiguration(MetricConfiguration config
configuration.getDescriptorStrategy(),
configuration.getResourceAttributesFilter(),
configuration.getUseServiceTimeSeries(),
configuration.getMonitoredResourceDescription());
configuration.getMonitoredResourceDescription(),
configuration.getInstrumentationLibraryLabelsEnabled());
}

@VisibleForTesting
Expand All @@ -114,15 +118,17 @@ static InternalMetricExporter createWithClient(
MetricDescriptorStrategy descriptorStrategy,
Predicate<AttributeKey<?>> resourceAttributesFilter,
boolean useCreateServiceTimeSeries,
MonitoredResourceDescription monitoredResourceDescription) {
MonitoredResourceDescription monitoredResourceDescription,
boolean instrumentationLibraryLabelsEnabled) {
return new InternalMetricExporter(
projectId,
prefix,
metricServiceClient,
descriptorStrategy,
resourceAttributesFilter,
useCreateServiceTimeSeries,
monitoredResourceDescription);
monitoredResourceDescription,
instrumentationLibraryLabelsEnabled);
}

private static MetricServiceSettings generateMetricServiceSettings(
Expand Down Expand Up @@ -177,7 +183,11 @@ public CompletableResultCode export(Collection<MetricData> metrics) {
// 3. Fire the set of time series off.
MetricTimeSeriesBuilder builder =
new AggregateByLabelMetricTimeSeriesBuilder(
projectId, prefix, resourceAttributesFilter, monitoredResourceDescription);
projectId,
prefix,
resourceAttributesFilter,
monitoredResourceDescription,
instrumentationLibraryLabelsEnabled);
for (final MetricData metricData : metrics) {
// Extract all the underlying points.
switch (metricData.getType()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ public abstract class MetricConfiguration {
* MetricConfiguration.Builder#setProjectId(String)}, this method returns a {@link Supplier} that
* supplies the default Project ID.
*
* @see ServiceOptions#getDefaultProjectId()
* @return a {@link Supplier} for the GCP project ID.
* @see ServiceOptions#getDefaultProjectId()
*/
abstract Supplier<String> getProjectIdSupplier();

Expand All @@ -104,10 +104,10 @@ public final String getProjectId() {
/**
* Returns the prefix prepended to metric names.
*
* @return the prefix to attach to metrics.
* @see <a href="https://cloud.google.com/monitoring/custom-metrics#identifier">Custom Metrics
* Identifiers</a>
* <p>Defaults to workload.googleapis.com.
* @return the prefix to attach to metrics.
*/
public abstract String getPrefix();

Expand Down Expand Up @@ -182,6 +182,15 @@ public final String getProjectId() {
@Nullable
public abstract MetricServiceSettings getMetricServiceSettings();

/**
* Returns a boolean indicating if the {@link MetricConfiguration} is configured to add
* instrumentation library labels to the metric attributes during export.
*
* @return true if the {@link MetricConfiguration} is configured to add instrumentation library
* labels to metrics, false otherwise.
*/
public abstract boolean getInstrumentationLibraryLabelsEnabled();

@VisibleForTesting
abstract boolean getInsecureEndpoint();

Expand All @@ -206,6 +215,7 @@ public static Builder builder() {
.setDescriptorStrategy(MetricDescriptorStrategy.SEND_ONCE)
.setInsecureEndpoint(false)
.setUseServiceTimeSeries(false)
.setInstrumentationLibraryLabelsEnabled(true)
.setResourceAttributesFilter(DEFAULT_RESOURCE_ATTRIBUTES_FILTER)
.setMonitoredResourceDescription(EMPTY_MONITORED_RESOURCE_DESCRIPTION)
.setMetricServiceEndpoint(DEFAULT_METRIC_SERVICE_ENDPOINT);
Expand Down Expand Up @@ -310,15 +320,37 @@ public abstract Builder setMonitoredResourceDescription(
* <li>{@link MetricConfiguration.Builder#setMetricServiceEndpoint(String)}
* </ul>
*
* The intended effect of setting these values in the configuration should instead be achieved
* by configuring the {@link MetricServiceSettings} object.
* <p>The intended effect of setting these values in the configuration should instead be
* achieved by configuring the {@link MetricServiceSettings} object.
*
* @param metricServiceSettings the {@link MetricServiceSettings} containing the configured
* options.
* @return this.
*/
public abstract Builder setMetricServiceSettings(MetricServiceSettings metricServiceSettings);

/**
* Sets the {@link MetricConfiguration} to configure the exporter to add instrumentation library
* labels as metric attributes during export.
*
* <p>Enabling instrumentation library labels adds the following metric attributes to exported
* metric data points:
*
* <ul>
* <li>instrumentation_source
* <li>instrumentation_version
* </ul>
*
* The value for these metric attributes is retrieved from {@link
* io.opentelemetry.sdk.common.InstrumentationScopeInfo}.
*
* @param instrumentationLibraryLabelsEnabled boolean indicating whether to add instrumentation
* library labels.
* @return this.
*/
public abstract Builder setInstrumentationLibraryLabelsEnabled(
boolean instrumentationLibraryLabelsEnabled);

@VisibleForTesting
abstract Builder setInsecureEndpoint(boolean value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ public void testExportSendsAllDescriptorsOnce() {
MetricDescriptorStrategy.SEND_ONCE,
DEFAULT_RESOURCE_ATTRIBUTES_FILTER,
false,
EMPTY_MONITORED_RESOURCE_DESCRIPTION);
EMPTY_MONITORED_RESOURCE_DESCRIPTION,
true);
CompletableResultCode result = exporter.export(ImmutableList.of(aMetricData, aHistogram));
assertTrue(result.isSuccess());
CompletableResultCode result2 = exporter.export(ImmutableList.of(aMetricData, aHistogram));
Expand Down Expand Up @@ -332,7 +333,8 @@ public void testExportSucceeds() {
MetricDescriptorStrategy.ALWAYS_SEND,
DEFAULT_RESOURCE_ATTRIBUTES_FILTER,
false,
EMPTY_MONITORED_RESOURCE_DESCRIPTION);
EMPTY_MONITORED_RESOURCE_DESCRIPTION,
true);

CompletableResultCode result = exporter.export(ImmutableList.of(aMetricData));
verify(mockClient, times(1)).createMetricDescriptor(metricDescriptorCaptor.capture());
Expand Down Expand Up @@ -455,7 +457,8 @@ public void testExportWithHistogram_Succeeds() {
MetricDescriptorStrategy.ALWAYS_SEND,
DEFAULT_RESOURCE_ATTRIBUTES_FILTER,
false,
EMPTY_MONITORED_RESOURCE_DESCRIPTION);
EMPTY_MONITORED_RESOURCE_DESCRIPTION,
true);
CompletableResultCode result = exporter.export(ImmutableList.of(aHistogram));
verify(mockClient, times(1)).createMetricDescriptor(metricDescriptorCaptor.capture());
verify(mockClient, times(1))
Expand All @@ -477,7 +480,8 @@ public void testExportWithNonSupportedMetricTypeReturnsFailure() {
MetricDescriptorStrategy.ALWAYS_SEND,
NO_RESOURCE_ATTRIBUTES,
false,
EMPTY_MONITORED_RESOURCE_DESCRIPTION);
EMPTY_MONITORED_RESOURCE_DESCRIPTION,
true);

MetricData metricData =
ImmutableMetricData.createDoubleSummary(
Expand Down Expand Up @@ -585,7 +589,8 @@ public void testExportWithMonitoredResourceMappingSucceeds() {
MetricDescriptorStrategy.ALWAYS_SEND,
customAttributesFilter,
false,
monitoredResourceDescription);
monitoredResourceDescription,
true);

CompletableResultCode result = exporter.export(ImmutableList.of(aMetricDataWithCustomResource));
verify(mockClient, times(1)).createMetricDescriptor(metricDescriptorCaptor.capture());
Expand Down Expand Up @@ -681,7 +686,8 @@ public void testExportWithMonitoredResourceMappingSucceeds_NoMRLabels() {
MetricDescriptorStrategy.ALWAYS_SEND,
customAttributesFilter,
false,
monitoredResourceDescription);
monitoredResourceDescription,
true);

CompletableResultCode result = exporter.export(ImmutableList.of(aMetricDataWithCustomResource));
verify(mockClient, times(1)).createMetricDescriptor(metricDescriptorCaptor.capture());
Expand All @@ -696,7 +702,8 @@ public void testExportWithMonitoredResourceMappingSucceeds_NoMRLabels() {
}

@Test
public void testExportWithMonitoredResourceMappingSucceeds_NoResourceLabels() {
public void
testExportWithMonitoredResourceMappingSucceeds_NoResourceLabels_NoInstrumentationLabels() {
MonitoredResourceDescription monitoredResourceDescription =
new MonitoredResourceDescription(
"custom_mr_instance", Set.of("service_instance_id", "host_id", "location"));
Expand Down Expand Up @@ -743,15 +750,14 @@ public void testExportWithMonitoredResourceMappingSucceeds_NoResourceLabels() {
.setValue(TypedValue.newBuilder().setInt64Value(aLongPoint.getValue()))
.setInterval(expectedTimeInterval)
.build();
// expected timeseries metric does not have the instrumentation library labels
TimeSeries expectedTimeSeries =
TimeSeries.newBuilder()
.setMetric(
Metric.newBuilder()
.setType(expectedDescriptor.getType())
.putLabels("label1", "value1")
.putLabels("label2", "false")
.putLabels(LABEL_INSTRUMENTATION_SOURCE, "instrumentName")
.putLabels(LABEL_INSTRUMENTATION_VERSION, "0")
.build())
.addPoints(expectedPoint)
.setMetricKind(expectedDescriptor.getMetricKind())
Expand All @@ -772,7 +778,8 @@ public void testExportWithMonitoredResourceMappingSucceeds_NoResourceLabels() {
MetricDescriptorStrategy.ALWAYS_SEND,
customAttributesFilter,
false,
monitoredResourceDescription);
monitoredResourceDescription,
false); // disable instrumentationLibrary labels generation

CompletableResultCode result =
exporter.export(ImmutableList.of(aMetricDataWithEmptyResourceAttributes));
Expand Down Expand Up @@ -849,7 +856,8 @@ public void verifyExporterExportGoogleServiceMetrics() {
MetricDescriptorStrategy.ALWAYS_SEND,
NO_RESOURCE_ATTRIBUTES,
true,
EMPTY_MONITORED_RESOURCE_DESCRIPTION);
EMPTY_MONITORED_RESOURCE_DESCRIPTION,
true);

CompletableResultCode result =
exporter.export(ImmutableList.of(googleComputeServiceMetricData));
Expand Down

0 comments on commit cfbc409

Please sign in to comment.