phet commented on code in PR #3933:
URL: https://github.com/apache/gobblin/pull/3933#discussion_r1577051274
##########
gobblin-metrics-libs/gobblin-metrics/src/main/java/org/apache/gobblin/metrics/OpenTelemetryMetrics.java:
##########
@@ -67,8 +74,9 @@ public static OpenTelemetryMetrics getInstance(State state) {
@Override
protected void initialize(State state) {
- Properties metricProps =
PropertiesUtils.extractPropertiesWithPrefix(state.getProperties(), Optional.of(
- ConfigurationKeys.METRICS_REPORTING_OPENTELEMETRY_CONFIGS));
+ log.info("Initializing OpenTelemetry metrics");
+ Properties metricProps =
PropertiesUtils.extractPropertiesWithPrefixAfterRemovingPrefix(state.getProperties(),
Review Comment:
wow, this name is wild! :)
`extractChildProperties`?
`extractRelativeChildren`?
##########
gobblin-metrics-libs/gobblin-metrics/src/main/java/org/apache/gobblin/metrics/OpenTelemetryMetrics.java:
##########
@@ -54,6 +57,10 @@ private OpenTelemetryMetrics(State state) {
protected MetricExporter initializeMetricExporter(State state) {
OtlpHttpMetricExporterBuilder httpExporterBuilder =
OtlpHttpMetricExporter.builder();
httpExporterBuilder.setEndpoint(state.getProp(ConfigurationKeys.METRICS_REPORTING_OPENTELEMETRY_ENDPOINT));
+ if
(state.contains(ConfigurationKeys.METRICS_REPORTING_OPENTELEMETRY_HEADER_KEY)) {
+
httpExporterBuilder.addHeader(state.getProp(ConfigurationKeys.METRICS_REPORTING_OPENTELEMETRY_HEADER_KEY),
+
state.getProp(ConfigurationKeys.METRICS_REPORTING_OPENTELEMETRY_HEADER_VALUE));
Review Comment:
do we really want to set it to `null` when both not provided?
##########
gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java:
##########
@@ -908,13 +908,19 @@ public class ConfigurationKeys {
public static final String METRICS_REPORTING_OPENTELEMETRY_ENABLED =
METRICS_CONFIGURATIONS_PREFIX + "reporting.opentelemtry.metrics.enabled";
- public static final String METRICS_REPORTING_OPENTELEMETRY_CONFIGS =
- METRICS_CONFIGURATIONS_PREFIX + "reporting.opentelemetry.configs";
+ public static final String METRICS_REPORTING_OPENTELEMETRY_CONFIGS_PREFIX =
+ METRICS_CONFIGURATIONS_PREFIX + "reporting.opentelemetry.configs.";
public static final Boolean DEFAULT_METRICS_REPORTING_OPENTELEMETRY_ENABLED
= false;
public static final String METRICS_REPORTING_OPENTELEMETRY_ENDPOINT =
METRICS_CONFIGURATIONS_PREFIX + "reporting.opentelemetry.endpoint";
+ public static final String METRICS_REPORTING_OPENTELEMETRY_HEADER_KEY =
+ METRICS_CONFIGURATIONS_PREFIX + "reporting.opentelemetry.header.key";
+
+ public static final String METRICS_REPORTING_OPENTELEMETRY_HEADER_VALUE =
+ METRICS_CONFIGURATIONS_PREFIX + "reporting.opentelemetry.header.value";
Review Comment:
don't these repeat `reporting.opentelemetry.` twice? is that intended?
##########
gobblin-service/src/test/java/org/apache/gobblin/service/monitoring/GaaSObservabilityProducerTest.java:
##########
@@ -54,6 +55,7 @@
import org.apache.gobblin.service.ServiceConfigKeys;
import org.apache.gobblin.service.modules.orchestration.AzkabanProjectConfig;
import org.apache.gobblin.service.modules.spec.JobExecutionPlan;
+import org.apache.gobblin.util.PropertiesUtils;
Review Comment:
did these two sneak in? not seeing where they're used...
--
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]
