From 0c824fed5a3e65fc5e7439e02a08c7ea5fee5455 Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Thu, 27 Jul 2023 13:05:11 -0400 Subject: [PATCH] Fix time inversion error when writing metrics (#2086) The instance ID used to be uniquely determined by App Engine SDK. Since we no longer calls the SDK, we need a way to distinguish instances so that their metrics would not stump on each other and result in a time inversion error (as we have seen frequently in the logs since the removal of the App Engine SDK). --- .../whitebox/StackdriverModule.java | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/google/registry/monitoring/whitebox/StackdriverModule.java b/core/src/main/java/google/registry/monitoring/whitebox/StackdriverModule.java index d5423395f..123900f11 100644 --- a/core/src/main/java/google/registry/monitoring/whitebox/StackdriverModule.java +++ b/core/src/main/java/google/registry/monitoring/whitebox/StackdriverModule.java @@ -25,17 +25,31 @@ import dagger.Module; import dagger.Provides; import google.registry.config.CredentialModule.ApplicationDefaultCredential; import google.registry.config.RegistryConfig.Config; +import google.registry.util.Clock; import google.registry.util.GoogleCredentialsBundle; +import javax.inject.Named; +import javax.inject.Singleton; import org.joda.time.Duration; /** Dagger module for Google Stackdriver service connection objects. */ @Module public final class StackdriverModule { + private StackdriverModule() {} + // We need a fake GCE zone to appease Stackdriver's resource model. // TODO(b/265973059): Switch to resource type "gke_container". private static final String SPOOFED_GCE_ZONE = "us-central1-f"; - private static final String SPOOFED_GCE_INSTANCE = "fake-instance"; + + // We cannot use a static fake intance ID which is shared by all instances, because metrics might + // be flushed to stackdriver with delays, which lead to time inversion erros when another instance + // has already written a data point at a later time. + @Singleton + @Provides + @Named("spoofedGceInstanceId") + static String providesSpoofedGceInstanceId(Clock clock) { + return clock.nowUtc().toString(); + } @Provides static Monitoring provideMonitoring( @@ -54,7 +68,8 @@ public final class StackdriverModule { Monitoring monitoringClient, @Config("projectId") String projectId, @Config("stackdriverMaxQps") int maxQps, - @Config("stackdriverMaxPointsPerRequest") int maxPointsPerRequest) { + @Config("stackdriverMaxPointsPerRequest") int maxPointsPerRequest, + @Named("spoofedGceInstanceId") String instanceId) { // The MonitoredResource for GAE apps is not writable (and missing fields anyway) so we just // use the gce_instance resource type instead. return new StackdriverWriter( @@ -65,7 +80,7 @@ public final class StackdriverModule { .setLabels( ImmutableMap.of( // The "zone" field MUST be a valid GCE zone, so we fake one. - "zone", SPOOFED_GCE_ZONE, "instance_id", SPOOFED_GCE_INSTANCE)), + "zone", SPOOFED_GCE_ZONE, "instance_id", instanceId)), maxQps, maxPointsPerRequest); }