rmetzger commented on code in PR #26850:
URL: https://github.com/apache/flink/pull/26850#discussion_r2245377565
##########
flink-metrics/flink-metrics-otel/src/main/java/org/apache/flink/metrics/otel/OpenTelemetryMetricReporter.java:
##########
@@ -92,10 +95,21 @@ public OpenTelemetryMetricReporter() {
public void open(MetricConfig metricConfig) {
LOG.info("Starting OpenTelemetryMetricReporter");
super.open(metricConfig);
- OtlpGrpcMetricExporterBuilder builder =
OtlpGrpcMetricExporter.builder();
- tryConfigureEndpoint(metricConfig, builder::setEndpoint);
- tryConfigureTimeout(metricConfig, builder::setTimeout);
- exporter = builder.build();
+
+ final String protocol =
+
metricConfig.getProperty(OpenTelemetryReporterOptions.EXPORTER_PROTOCOL.key());
+
+ if (Protocol.HTTP.name().equalsIgnoreCase(protocol)) {
+ OtlpHttpMetricExporterBuilder builder =
OtlpHttpMetricExporter.builder();
+ tryConfigureEndpoint(metricConfig, builder::setEndpoint);
+ tryConfigureTimeout(metricConfig, builder::setTimeout);
+ exporter = builder.build();
+ } else {
+ OtlpGrpcMetricExporterBuilder builder =
OtlpGrpcMetricExporter.builder();
+ tryConfigureEndpoint(metricConfig, builder::setEndpoint);
+ tryConfigureTimeout(metricConfig, builder::setTimeout);
+ exporter = builder.build();
Review Comment:
Same issue as with the event reporter
##########
flink-metrics/flink-metrics-otel/src/test/java/org/apache/flink/metrics/otel/OtelTestContainer.java:
##########
@@ -31,7 +31,7 @@
import java.util.Locale;
/** {@link OtelTestContainer} provides an {@code Otel} test instance. */
-class OtelTestContainer extends GenericContainer<OtelTestContainer> {
+public class OtelTestContainer extends GenericContainer<OtelTestContainer> {
Review Comment:
Where are you using this class for testing? Will there be a follow up PR?
##########
flink-metrics/flink-metrics-otel/src/main/java/org/apache/flink/events/otel/OpenTelemetryEventReporter.java:
##########
@@ -60,11 +64,21 @@ public OpenTelemetryEventReporter() {
@Override
public void open(MetricConfig metricConfig) {
LOG.info("Starting OpenTelemetryEventReporter");
- OtlpGrpcLogRecordExporterBuilder builder =
OtlpGrpcLogRecordExporter.builder();
- tryConfigureEndpoint(metricConfig, builder::setEndpoint);
- tryConfigureTimeout(metricConfig, builder::setTimeout);
+ final String protocol =
+
metricConfig.getProperty(OpenTelemetryReporterOptions.EXPORTER_PROTOCOL.key());
+
+ if (Protocol.HTTP.name().equalsIgnoreCase(protocol)) {
+ OtlpHttpLogRecordExporterBuilder builder =
OtlpHttpLogRecordExporter.builder();
+ tryConfigureEndpoint(metricConfig, builder::setEndpoint);
+ tryConfigureTimeout(metricConfig, builder::setTimeout);
+ logRecordExporter = builder.build();
+ } else {
+ OtlpGrpcLogRecordExporterBuilder builder =
OtlpGrpcLogRecordExporter.builder();
+ tryConfigureEndpoint(metricConfig, builder::setEndpoint);
+ tryConfigureTimeout(metricConfig, builder::setTimeout);
+ logRecordExporter = builder.build();
+ }
Review Comment:
I think it would be a better UX to at least log a warning if we see an
unexpected protocol, instead of just silently doing grpc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
