Repository: GoogleCloudPlatform/pgadapter
Pull request #1629 (2024-04-18 09:29:53)

fix: add a random service name for OpenTelemetry
Adds a random number after the standard pgadapter service name used for OpenTelemetry, and adds this to the Metrics exporter. This prevents the `One or more TimeSeries could not be written: Points must be written in order.` error.


Patch:
diff --git a/pom.xml b/pom.xml
index fc79fb45..aa963873 100644
--- a/pom.xml
+++ b/pom.xml
@@ -195,7 +195,7 @@
     <dependency>
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-text</artifactId>
-      <version>1.11.0</version>
+      <version>1.12.0</version>
     </dependency>
     <dependency>
       <groupId>org.apache.maven</groupId>
diff --git a/samples/python/psycopg3/requirements.txt b/samples/python/psycopg3/requirements.txt
index 553878d3..a8cf7314 100644
--- a/samples/python/psycopg3/requirements.txt
+++ b/samples/python/psycopg3/requirements.txt
@@ -1,4 +1,4 @@
 psycopg~=3.1.18
-testcontainers~=4.3.3
+testcontainers~=4.4.0
 google~=3.0.0
 google.auth~=2.29.0
\ No newline at end of file
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/Server.java b/src/main/java/com/google/cloud/spanner/pgadapter/Server.java
index 56b5d37c..f0617ddb 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/Server.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/Server.java
@@ -14,6 +14,8 @@
 
 package com.google.cloud.spanner.pgadapter;
 
+import static io.opentelemetry.semconv.ServiceAttributes.SERVICE_NAME;
+
 import com.google.auth.Credentials;
 import com.google.cloud.opentelemetry.metric.GoogleCloudMetricExporter;
 import com.google.cloud.opentelemetry.metric.MetricConfiguration;
@@ -24,10 +26,12 @@ import com.google.common.collect.ImmutableMap;
 import com.google.devtools.cloudtrace.v2.AttributeValue;
 import com.google.devtools.cloudtrace.v2.TruncatableString;
 import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder;
 import io.opentelemetry.sdk.metrics.export.MetricExporter;
 import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader;
+import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
 import io.opentelemetry.sdk.trace.export.SpanExporter;
 import io.opentelemetry.sdk.trace.samplers.Sampler;
@@ -38,6 +42,7 @@ import java.io.PrintStream;
 import java.time.Duration;
 import java.util.Locale;
 import java.util.Objects;
+import java.util.concurrent.ThreadLocalRandom;
 import org.apache.maven.model.Model;
 import org.apache.maven.model.io.xpp3.MavenXpp3Reader;
 
@@ -76,8 +81,9 @@ public class Server {
       System.setProperty("otel.logs.exporter", "none");
     }
     if (getOpenTelemetrySetting("otel.service.name") == null) {
-      System.setProperty("otel.service.name", "pgadapter");
+      System.setProperty("otel.service.name", "pgadapter-" + ThreadLocalRandom.current().nextInt());
     }
+    String serviceName = Objects.requireNonNull(getOpenTelemetrySetting("otel.service.name"));
 
     try {
       String projectId = optionsMetadata.getTelemetryProjectId();
@@ -95,14 +101,9 @@ public class Server {
         }
         builder.setFixedAttributes(
             ImmutableMap.of(
-                "service.name",
+                SERVICE_NAME.getKey(),
                 AttributeValue.newBuilder()
-                    .setStringValue(
-                        TruncatableString.newBuilder()
-                            .setValue(
-                                Objects.requireNonNull(
-                                    getOpenTelemetrySetting("otel.service.name")))
-                            .build())
+                    .setStringValue(TruncatableString.newBuilder().setValue(serviceName).build())
                     .build()));
         TraceConfiguration configuration = builder.build();
         SpanExporter traceExporter = TraceExporter.createWithConfiguration(configuration);
@@ -131,8 +132,10 @@ public class Server {
                     .build());
         openTelemetryBuilder.addMeterProviderCustomizer(
             (sdkMeterProviderBuilder, configProperties) ->
-                sdkMeterProviderBuilder.registerMetricReader(
-                    PeriodicMetricReader.builder(cloudMonitoringExporter).build()));
+                sdkMeterProviderBuilder
+                    .addResource(Resource.create(Attributes.of(SERVICE_NAME, serviceName)))
+                    .registerMetricReader(
+                        PeriodicMetricReader.builder(cloudMonitoringExporter).build()));
       }
       return openTelemetryBuilder.build().getOpenTelemetrySdk();
     } catch (IOException exception) {
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/statements/ExtendedQueryProtocolHandler.java b/src/main/java/com/google/cloud/spanner/pgadapter/statements/ExtendedQueryProtocolHandler.java
index 1cef475e..3c74fce1 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/statements/ExtendedQueryProtocolHandler.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/statements/ExtendedQueryProtocolHandler.java
@@ -66,7 +66,9 @@ public class ExtendedQueryProtocolHandler {
                 .getServer()
                 .getTracer(ConnectionHandler.class.getName(), getVersion()),
             connectionHandler.getServer().getMetrics(),
-            createMetricAttributes(connectionHandler.getDatabaseId()),
+            createMetricAttributes(
+                connectionHandler.getDatabaseId(),
+                connectionHandler.getTraceConnectionId().toString()),
             connectionHandler.getTraceConnectionId().toString(),
             connectionHandler::closeAllPortals,
             connectionHandler.getDatabaseId(),
@@ -95,8 +97,9 @@ public class ExtendedQueryProtocolHandler {
   }
 
   @VisibleForTesting
-  static Attributes createMetricAttributes(DatabaseId databaseId) {
+  static Attributes createMetricAttributes(DatabaseId databaseId, String connectionId) {
     AttributesBuilder attributesBuilder = Attributes.builder();
+    attributesBuilder.put("pgadapter.connection_id", connectionId);
     attributesBuilder.put("database", databaseId.getDatabase());
     attributesBuilder.put("instance_id", databaseId.getInstanceId().getInstance());
     attributesBuilder.put("project_id", databaseId.getInstanceId().getProject());


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/statements/BackendConnectionTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/statements/BackendConnectionTest.java
index d1809527..f56f53e2 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/statements/BackendConnectionTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/statements/BackendConnectionTest.java
@@ -85,7 +85,7 @@ public class BackendConnectionTest {
   private static final Runnable DO_NOTHING = () -> {};
   private static final DatabaseId DATABASE_ID = DatabaseId.of("p", "i", "d");
   private static final Attributes METRIC_ATTRIBUTES =
-      ExtendedQueryProtocolHandler.createMetricAttributes(DATABASE_ID);
+      ExtendedQueryProtocolHandler.createMetricAttributes(DATABASE_ID, "test-id");
 
   @Test
   public void testExtractDdlUpdateCounts() {
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java
index a5513ef5..9cac0270 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java
@@ -97,7 +97,7 @@ public class StatementTest {
   private static final Runnable DO_NOTHING = () -> {};
   private static final DatabaseId DATABASE_ID = DatabaseId.of("p", "i", "d");
   private static final Attributes METRIC_ATTRIBUTES =
-      ExtendedQueryProtocolHandler.createMetricAttributes(DATABASE_ID);
+      ExtendedQueryProtocolHandler.createMetricAttributes(DATABASE_ID, "test-connection");
 
   private static ParsedStatement parse(String sql) {
     return PARSER.parse(Statement.of(sql));
@@ -521,7 +521,7 @@ public class StatementTest {
         new BackendConnection(
             NOOP_OTEL,
             NOOP_OTEL_METER,
-            ExtendedQueryProtocolHandler.createMetricAttributes(databaseId),
+            ExtendedQueryProtocolHandler.createMetricAttributes(databaseId, "test-connection"),
             UUID.randomUUID().toString(),
             DO_NOTHING,
             databaseId,
@@ -629,7 +629,7 @@ public class StatementTest {
         new BackendConnection(
             NOOP_OTEL,
             NOOP_OTEL_METER,
-            ExtendedQueryProtocolHandler.createMetricAttributes(databaseId),
+            ExtendedQueryProtocolHandler.createMetricAttributes(databaseId, "test-connection"),
             UUID.randomUUID().toString(),
             DO_NOTHING,
             databaseId,
@@ -688,7 +688,7 @@ public class StatementTest {
         new BackendConnection(
             NOOP_OTEL,
             NOOP_OTEL_METER,
-            ExtendedQueryProtocolHandler.createMetricAttributes(databaseId),
+            ExtendedQueryProtocolHandler.createMetricAttributes(databaseId, "test-connection"),
             UUID.randomUUID().toString(),
             DO_NOTHING,
             databaseId,