phet commented on code in PR #3933:
URL: https://github.com/apache/gobblin/pull/3933#discussion_r1577237486
##########
gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/GaaSObservabilityEventProducer.java:
##########
@@ -99,17 +98,18 @@ protected OpenTelemetryMetricsBase
getOpentelemetryMetrics(State state) {
private void setupMetrics(State state) {
this.opentelemetryMetrics = getOpentelemetryMetrics(state);
if (this.opentelemetryMetrics != null) {
- this.jobStatusMetric =
this.opentelemetryMetrics.getMeter(state.getProp(GAAS_OBSERVABILITY_GROUP_NAME))
+ this.jobStatusMetric =
this.opentelemetryMetrics.getMeter(GAAS_OBSERVABILITY_METRICS_GROUPNAME)
.gaugeBuilder(GAAS_OBSERVABILITY_JOB_STATUS_METRIC_NAME)
.ofLongs()
.buildObserver();
-
this.opentelemetryMetrics.getMeter(state.getProp(GAAS_OBSERVABILITY_GROUP_NAME))
+ this.opentelemetryMetrics.getMeter(GAAS_OBSERVABILITY_METRICS_GROUPNAME)
.batchCallback(() -> {
for (GaaSObservabilityEventExperimental event :
this.eventCollector) {
Attributes tags = getEventAttributes(event);
int status = event.getJobStatus() != JobStatus.SUCCEEDED ? 1 : 0;
this.jobStatusMetric.record(status, tags);
}
+ log.info("Submitted {} job status metrics",
this.eventCollector.size());
Review Comment:
nit: I'd call these "job status events", not "merics"
##########
gobblin-metrics-libs/gobblin-metrics/src/main/java/org/apache/gobblin/metrics/OpenTelemetryMetrics.java:
##########
@@ -42,18 +44,26 @@
* can replace the old metrics system with tighter integrations once it's
stable
*/
+@Slf4j
public class OpenTelemetryMetrics extends OpenTelemetryMetricsBase {
private static OpenTelemetryMetrics GLOBAL_INSTANCE;
private static final Long DEFAULT_OPENTELEMETRY_REPORTING_INTERVAL_MILLIS =
10000L;
+
private OpenTelemetryMetrics(State state) {
super(state);
}
@Override
protected MetricExporter initializeMetricExporter(State state) {
+
Preconditions.checkArgument(state.contains(ConfigurationKeys.METRICS_REPORTING_OPENTELEMETRY_ENDPOINT),
Review Comment:
wondering: who calls this? i.e. when the precondition throws, who's getting
the exception?
...and are they ready, such that this wouldn't just silently kill some
metrics reporting thread w/ no indication other than perhaps a message in our
system log?
##########
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:
good call. thinking defaults here: is there a canonical value OT would want
or it's deployment-specific, such as perhaps an endpoint URL?
--
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]
