Dont use globals for the tc metrics settings (#343)

OctopusDeploy · Dec 3, 2024 · 5641021 · 5641021
1 parent 72de115
commit 5641021
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 56 deletions.
diff --git a/...c/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/IOTELEndpointHandler.java b/...c/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/IOTELEndpointHandler.java
@@ -1,9 +1,11 @@
 package com.octopus.teamcity.opentelemetry.server.endpoints;
 
 import com.octopus.teamcity.opentelemetry.server.SetProjectConfigurationSettingsRequest;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 import jetbrains.buildServer.serverSide.BuildPromotion;
 import jetbrains.buildServer.serverSide.SBuild;
+import org.apache.commons.lang3.tuple.Pair;
 import org.springframework.web.servlet.ModelAndView;
 
 import javax.servlet.http.HttpServletRequest;
@@ -12,7 +14,7 @@
 public interface IOTELEndpointHandler {
     ModelAndView getBuildOverviewModelAndView(SBuild build, Map<String, String> params, String traceId);
 
-    SpanProcessor buildSpanProcessor(BuildPromotion buildPromotion, String endpoint, Map<String, String> params);
+    Pair<SpanProcessor, SdkMeterProvider> buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map<String, String> params);
 
     SetProjectConfigurationSettingsRequest getSetProjectConfigurationSettingsRequest(HttpServletRequest request);
 

diff --git a/...com/octopus/teamcity/opentelemetry/server/endpoints/custom/CustomOTELEndpointHandler.java b/...com/octopus/teamcity/opentelemetry/server/endpoints/custom/CustomOTELEndpointHandler.java
@@ -4,13 +4,15 @@
 import com.octopus.teamcity.opentelemetry.server.endpoints.IOTELEndpointHandler;
 import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter;
 import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporterBuilder;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
 import io.opentelemetry.sdk.trace.export.SpanExporter;
 import jetbrains.buildServer.serverSide.BuildPromotion;
 import jetbrains.buildServer.serverSide.SBuild;
 import jetbrains.buildServer.serverSide.crypt.EncryptUtil;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.log4j.Logger;
 import org.springframework.web.servlet.ModelAndView;
 
@@ -35,7 +37,7 @@ public ModelAndView getBuildOverviewModelAndView(SBuild build, Map<String, Strin
     }
 
     @Override
-    public SpanProcessor buildSpanProcessor(BuildPromotion buildPromotion, String endpoint, Map<String, String> params) {
+    public Pair<SpanProcessor, SdkMeterProvider> buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map<String, String> params) {
         Map<String, String> headers = new HashMap<>();
         params.forEach((k, v) -> {
             if (k.startsWith(PROPERTY_KEY_HEADERS)) {
@@ -45,7 +47,7 @@ public SpanProcessor buildSpanProcessor(BuildPromotion buildPromotion, String en
                 headers.put(name, value);
             }
         });
-        return buildGrpcSpanProcessor(headers, endpoint);
+        return Pair.of(buildGrpcSpanProcessor(headers, endpoint), null);
     }
 
     @Override

diff --git a/...topus/teamcity/opentelemetry/server/endpoints/honeycomb/HoneycombOTELEndpointHandler.java b/...topus/teamcity/opentelemetry/server/endpoints/honeycomb/HoneycombOTELEndpointHandler.java
@@ -8,22 +8,25 @@
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter;
 import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.metrics.export.MetricExporter;
 import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
+import jetbrains.buildServer.serverSide.BuildPromotion;
 import io.opentelemetry.semconv.ServiceAttributes;
 import jetbrains.buildServer.serverSide.BuildPromotion;
 import jetbrains.buildServer.serverSide.SBuild;
 import jetbrains.buildServer.serverSide.TeamCityNodes;
 import jetbrains.buildServer.serverSide.crypt.EncryptUtil;
 import jetbrains.buildServer.serverSide.crypt.RSACipher;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.springframework.web.servlet.ModelAndView;
 
-import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 import java.util.Date;
 import java.util.HashMap;
@@ -63,7 +66,7 @@ public ModelAndView getBuildOverviewModelAndView(SBuild build, Map<String, Strin
     }
 
     @Override
-    public SpanProcessor buildSpanProcessor(BuildPromotion buildPromotion, String endpoint, Map<String, String> params) {
+    public Pair<SpanProcessor, SdkMeterProvider> buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map<String, String> params) {
         Map<String, String> headers = new HashMap<>();
         //todo: add a setting to say "use classic" or "use environments"
         headers.put("x-honeycomb-dataset", params.get(PROPERTY_KEY_HONEYCOMB_DATASET));
@@ -86,7 +89,7 @@ private MetricExporter buildMetricsExporter(String endpoint, Map<String, String>
         return null;
     }
 
-    private SpanProcessor buildGrpcSpanProcessor(
+    private Pair<SpanProcessor, SdkMeterProvider> buildGrpcSpanProcessor(
             BuildPromotion buildPromotion,
             Map<String, String> headers,
             String exporterEndpoint,
@@ -99,22 +102,24 @@ private SpanProcessor buildGrpcSpanProcessor(
                         AttributeKey.stringKey("teamcity.build_promotion.id"), Long.toString(buildPromotion.getId()),
                         AttributeKey.stringKey("teamcity.node.id"), nodesService.getCurrentNode().getId()
                 ));
-
         var meterProvider = OTELMetrics.getOTELMeterProvider(metricsExporter, serviceNameResource);
 
         var spanExporterBuilder = OtlpGrpcSpanExporter.builder();
+        spanExporterBuilder.setEndpoint(exporterEndpoint);
         headers.forEach(spanExporterBuilder::addHeader);
-        var spanExporter = spanExporterBuilder
-                .setEndpoint(exporterEndpoint)
-                .setMeterProvider(meterProvider)
-                .build();
-
-        return BatchSpanProcessor.builder(spanExporter)
-                .setMaxQueueSize(BATCH_SPAN_PROCESSOR_MAX_QUEUE_SIZE)
-                .setScheduleDelay(BATCH_SPAN_PROCESSOR_MAX_SCHEDULE_DELAY)
-                .setMaxExportBatchSize(BATCH_SPAN_PROCESSOR_MAX_EXPORT_BATCH_SIZE)
-                .setMeterProvider(meterProvider)
-                .build();
+        if (meterProvider != null) {
+            spanExporterBuilder.setMeterProvider(meterProvider);
+        }
+        var spanExporter = spanExporterBuilder.build();
+
+        var batchSpanProcessorBuilder = BatchSpanProcessor.builder(spanExporter);
+        batchSpanProcessorBuilder.setMaxQueueSize(BATCH_SPAN_PROCESSOR_MAX_QUEUE_SIZE);
+        batchSpanProcessorBuilder.setScheduleDelay(BATCH_SPAN_PROCESSOR_MAX_SCHEDULE_DELAY);
+        batchSpanProcessorBuilder.setMaxExportBatchSize(BATCH_SPAN_PROCESSOR_MAX_EXPORT_BATCH_SIZE);
+        if (meterProvider != null) {
+            batchSpanProcessorBuilder.setMeterProvider(meterProvider);
+        }
+        return Pair.of(batchSpanProcessorBuilder.build(), meterProvider);
     }
 
     @Override

diff --git a/...com/octopus/teamcity/opentelemetry/server/endpoints/zipkin/ZipkinOTELEndpointHandler.java b/...com/octopus/teamcity/opentelemetry/server/endpoints/zipkin/ZipkinOTELEndpointHandler.java
@@ -3,11 +3,13 @@
 import com.octopus.teamcity.opentelemetry.server.SetProjectConfigurationSettingsRequest;
 import com.octopus.teamcity.opentelemetry.server.endpoints.IOTELEndpointHandler;
 import io.opentelemetry.exporter.zipkin.ZipkinSpanExporter;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
 import jetbrains.buildServer.serverSide.BuildPromotion;
 import jetbrains.buildServer.serverSide.SBuild;
 import jetbrains.buildServer.web.openapi.PluginDescriptor;
+import org.apache.commons.lang3.tuple.Pair;
 import org.jetbrains.annotations.NotNull;
 import org.springframework.web.servlet.ModelAndView;
 
@@ -35,8 +37,8 @@ public ModelAndView getBuildOverviewModelAndView(SBuild build, Map<String, Strin
     }
 
     @Override
-    public SpanProcessor buildSpanProcessor(BuildPromotion buildPromotion, String endpoint, Map<String, String> params) {
-        return buildZipkinSpanProcessor(endpoint);
+    public Pair<SpanProcessor, SdkMeterProvider> buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map<String, String> params) {
+        return Pair.of(buildZipkinSpanProcessor(endpoint), null);
     }
 
     private SpanProcessor buildZipkinSpanProcessor(String exporterEndpoint) {

diff --git a/...va/com/octopus/teamcity/opentelemetry/server/helpers/HelperPerBuildOTELHelperFactory.java b/...va/com/octopus/teamcity/opentelemetry/server/helpers/HelperPerBuildOTELHelperFactory.java
@@ -1,6 +1,7 @@
 package com.octopus.teamcity.opentelemetry.server.helpers;
 
 import com.octopus.teamcity.opentelemetry.server.endpoints.OTELEndpointFactory;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 import jetbrains.buildServer.serverSide.BuildPromotion;
 import jetbrains.buildServer.serverSide.ProjectManager;
@@ -43,13 +44,14 @@ public OTELHelper getOTELHelper(BuildPromotion buildPromotion) {
                 var params = feature.getParameters();
                 if (params.get(PROPERTY_KEY_ENABLED).equals("true")) {
                     var endpoint = params.get(PROPERTY_KEY_ENDPOINT);
-                    SpanProcessor spanProcessor;
 
                     var otelHandler = otelEndpointFactory.getOTELEndpointHandler(params.get(PROPERTY_KEY_SERVICE));
-                    spanProcessor = otelHandler.buildSpanProcessor(buildPromotion, endpoint, params);
+                    var spanProcessorMeterProviderPair = otelHandler.buildSpanProcessorAndMeterProvider(buildPromotion, endpoint, params);
 
                     long startTime = System.nanoTime();
-                    var otelHelper = new OTELHelperImpl(spanProcessor, String.valueOf(buildId));
+                    var spanProcessor = spanProcessorMeterProviderPair.getLeft();
+                    var meterProvider = spanProcessorMeterProviderPair.getRight();
+                    var otelHelper = new OTELHelperImpl(spanProcessor, meterProvider, String.valueOf(buildId));
                     long endTime = System.nanoTime();
 
                     long duration = (endTime - startTime);

diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELHelperImpl.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELHelperImpl.java
@@ -9,6 +9,7 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
 import io.opentelemetry.sdk.trace.SpanProcessor;
@@ -26,8 +27,14 @@ public class OTELHelperImpl implements OTELHelper {
     private final ConcurrentHashMap<String, Span> spanMap;
     private final SdkTracerProvider sdkTracerProvider;
     private final String helperName;
+    @Nullable
+    private final SdkMeterProvider meterProvider;
 
-    public OTELHelperImpl(SpanProcessor spanProcessor, String helperName) {
+    public OTELHelperImpl(
+            SpanProcessor spanProcessor,
+            @Nullable
+            SdkMeterProvider meterProvider,
+            String helperName) {
         this.helperName = helperName;
         Resource serviceNameResource = Resource
                 .create(Attributes.of(ServiceAttributes.SERVICE_NAME, PluginConstants.SERVICE_NAME));
@@ -41,6 +48,7 @@ public OTELHelperImpl(SpanProcessor spanProcessor, String helperName) {
                 .build();
         this.tracer = this.openTelemetry.getTracer(PluginConstants.TRACER_INSTRUMENTATION_NAME);
         this.spanMap = new ConcurrentHashMap<>();
+        this.meterProvider = meterProvider;
     }
 
     @Override
@@ -92,6 +100,8 @@ public void release(String helperName) {
 
         this.sdkTracerProvider.forceFlush();
         this.sdkTracerProvider.close();
+        if (this.meterProvider != null)
+            this.meterProvider.close();
         this.spanMap.clear();
     }
 }
diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELMetrics.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELMetrics.java
@@ -1,48 +1,26 @@
 package com.octopus.teamcity.opentelemetry.server.helpers;
 
-import io.opentelemetry.api.GlobalOpenTelemetry;
-import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.metrics.export.MetricExporter;
 import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader;
 import io.opentelemetry.sdk.resources.Resource;
 
 import javax.annotation.Nullable;
 import java.time.Duration;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 public class OTELMetrics {
 
-    private static final AtomicBoolean metricsConfigured = new AtomicBoolean(false);
-    private static SdkMeterProvider sdkMeterProvider;
-
-    // this is not quite right...
-    // it's using project level settings (which can be different across projects)
-    // but setting up a global configuration.
-    // likely need to refactor the config so that it has metrics configuration as global
-
+    @Nullable
     public static SdkMeterProvider getOTELMeterProvider(@Nullable MetricExporter metricExporter, Resource serviceNameResource) {
-        if (metricsConfigured.get()) return sdkMeterProvider;
-        metricsConfigured.set(true);
+        if (metricExporter == null) return null;
 
-        var meterProviderBuilder = SdkMeterProvider.builder()
-                .setResource(Resource.getDefault().merge(serviceNameResource));
-        if (metricExporter != null) {
-            var providedMetricExporterBuilder = PeriodicMetricReader.builder(metricExporter)
-                    .setInterval(Duration.ofSeconds(10))
-                    .build();
-            meterProviderBuilder
-                    .registerMetricReader(providedMetricExporterBuilder);
-        }
-        sdkMeterProvider = meterProviderBuilder.build();
-        var globalOpenTelemetry = OpenTelemetrySdk.builder()
-                .setMeterProvider(sdkMeterProvider)
+        var providedMetricExporter = PeriodicMetricReader.builder(metricExporter)
+                .setInterval(Duration.ofSeconds(10))
                 .build();
 
-        GlobalOpenTelemetry.set(globalOpenTelemetry);
-
-        // Shutdown hooks to close resources properly
-        Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close));
-        return sdkMeterProvider;
+        return SdkMeterProvider.builder()
+                .setResource(Resource.getDefault().merge(serviceNameResource))
+                .registerMetricReader(providedMetricExporter)
+                .build();
     }
 }
diff --git a/server/src/test/java/com/octopus/teamcity/opentelemetry/server/OTELHelperTest.java b/server/src/test/java/com/octopus/teamcity/opentelemetry/server/OTELHelperTest.java
@@ -32,7 +32,7 @@ class OTELHelperTest {
     @BeforeEach
     void setUp() {
         GlobalOpenTelemetry.resetForTest();
-        this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), "helperNamr");
+        this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), null, "helperNamr");
     }
 
 

diff --git a/...er/src/test/java/com/octopus/teamcity/opentelemetry/server/TeamCityBuildListenerTest.java b/...er/src/test/java/com/octopus/teamcity/opentelemetry/server/TeamCityBuildListenerTest.java
@@ -33,7 +33,7 @@ class TeamCityBuildListenerTest {
     @BeforeEach
     void setUp(@Mock EventDispatcher<BuildServerListener> buildServerListenerEventDispatcher) {
         GlobalOpenTelemetry.resetForTest();
-        this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), "helper");
+        this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), null, "helper");
         this.factory = mock(OTELHelperFactory.class, RETURNS_DEEP_STUBS);
 
         var buildStorageManager = mock(BuildStorageManager.class, RETURNS_DEEP_STUBS);