"Failed to publish metrics to OTLP receiver" error message contains no actionable context

[climategadgets]

Describe the bug
Under some circumstances, a message Failed to publish metrics to OTLP receiver with an exception will be logged, usually observed with java.net.ConnectException: Connection refused as a root cause.

This message does not contain any information that allows to determine where exactly the meter registry is trying to connect, or any other relevant context, for that matter. Since it is possible that the registry is being automatically instantiated by frameworks (Quarkus, Spring, etc.) and that process is usually not logged, the only way to fix the problem is to guess what configuration item needs to be fixed.

Environment

Environment agnostic, here:

micrometer/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java

Line 182 in 061dbf6

logger.warn("Failed to publish metrics to OTLP receiver", e);

To Reproduce
How to reproduce the bug:

Have the framework instantiate the registry with no configuration present, or wrong configuration

Expected behavior
Error message must contain enough information to determine what needs to be changed to fix the problem.

• Minimum: Connection to $host:$port refused
• Preferred: Connection to $host:$port refused, check the value of x.y.z configuration keyword

It is understood that "connection refused" may not be the only root cause here. This doesn't change the problem definition - not enough context information to fix the problem.

[shakuzen]

Thank you for the detailed issue. It sounds like a good enhancement to me. Would you be up for making a pull request for it?

[climategadgets]

Thank you for the detailed issue. It sounds like a good enhancement to me. Would you be up for making a pull request for it?

Let me see what I can do.

[climategadgets]

Below is a quick fix; sure thing it needs test cases but OtlpMeterRegistry is pretty closed up (httpSender, the main source of problems, can't be easily injected) so I wanted to make sure that this goes in the direction you guys approve before investing time in tests.

Please advise.

diff --git a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java
index ef3eb90d1..0131c2565 100644
--- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java
+++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java
@@ -174,16 +174,28 @@ public class OtlpMeterRegistry extends PushMeterRegistry {
                 this.config.headers().forEach(httpRequest::withHeader);
                 HttpSender.Response response = httpRequest.send();
                 if (!response.isSuccessful()) {
-                    logger.warn("Failed to publish metrics. Server responded with HTTP status code {} and body {}",
+                    logger.warn("Failed to publish metrics (context: {}). Server responded with HTTP status code {} and body {}",
+                            getConfigurationContext(),
                             response.code(), response.body());
                 }
             }
             catch (Throwable e) {
-                logger.warn("Failed to publish metrics to OTLP receiver", e);
+                logger.warn("Failed to publish metrics to OTLP receiver (context: {})", getConfigurationContext(), e);
             }
         }
     }
 
+    /**
+     * Get the configuration context.
+     *
+     * @return A message containing enough information for the log reader to figure out what configuration details
+     * may have contributed to the failure.
+     */
+    private String getConfigurationContext() {
+        // While other values may contribute to failures, these two are most common
+        return "url=" + config.url() + ", resource-attributes=" + config.resourceAttributes();
+    }
+
     @Override
     protected <T> Gauge newGauge(Meter.Id id, @Nullable T obj, ToDoubleFunction<T> valueFunction) {
         return new DefaultGauge<>(id, obj, valueFunction);

[shakuzen]

Looks generally good to me. Logging the response body at warn level could be a bit verbose, though.

[climategadgets]

I thought I'm logging resource attributes, not response body - am I missing anything?

[climategadgets]

Meanwhile, another diff. sayHello() should be ideally an abstract method called from either PushMeterRegistry#start() or possibly from even MeterRegistry constructor (config is available there) so that subclasses can override it with their relevant details. Not knowing where the metrics are published is a very frequent complaint in my experience.

--- a/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java
+++ b/implementations/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMeterRegistry.java
@@ -135,6 +135,17 @@ public class OtlpMeterRegistry extends PushMeterRegistry {
             this.meterPollingService.scheduleAtFixedRate(this::pollMetersToRollover, getInitialDelay(),
                     config.step().toMillis(), TimeUnit.MILLISECONDS);
         }
+
+        if (config.enabled()) {
+            sayHello();
+        }
+    }
+
+    private void sayHello() {
+        logger.info("Publishing metrics for {} to {} with resource attributes {}",
+            getClass().getSimpleName(),
+            config.url(),
+            config.resourceAttributes());
     }

[shakuzen]

I thought I'm logging resource attributes, not response body - am I missing anything?

Sorry, I mixed up the existing code with what you added in the diff. It looks good to me.

For the other diff, let's separate that into another issue so we can track and merge them separately so one doesn't block progress on the other.

[jonatan-ivanov]

@climategadgets Would you mind creating a PR?
We can apply the patches you sent as comments above but it would be more trackable if you could open a PR.

[climategadgets]

@climategadgets Would you mind creating a PR? We can apply the patches you sent as comments above but it would be more trackable if you could open a PR.

Of course, will do.

[shakuzen]

1.13.0-M2 is planned for March 11. It would be nice to include this in that pre-release so users can test it out and give feedback.

[climategadgets]

1.13.0-M2 is planned for March 11. It would be nice to include this in that pre-release so users can test it out and give feedback.

Hopefully, tonight.

[climategadgets]

For the other diff, let's separate that into another issue so we can track and merge them separately so one doesn't block progress on the other.

#4828