[GOBBLIN-2209] Emit GaaS Executor Otel Metrics

[Blazer-007]

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

This pull request introduces new support for OpenTelemetry-based metrics in Gobblin, with a focus on extensibility and configurability for metric reporting. It adds configuration options, implements new metric types and helpers, and introduces standardized metric names and dimensions for Gobblin jobs.

The most important changes are:

OpenTelemetry Metrics Configuration:

• Added new configuration keys in ConfigurationKeys for OpenTelemetry metrics, including class name, enablement flag, histogram settings, dimensions, and group name. Also fixed the interval millis key and provided default values. [1] [2]

OpenTelemetry Metrics Implementation:

• Updated OpenTelemetryMetrics to use a thread-safe singleton pattern, allow configuration of histogram aggregation (max buckets and scale), and register a histogram view with the meter provider. [1] [2] [3] [4]
• Added imports for new OpenTelemetry SDK classes to support histogram aggregation and instrument selection.

New Metric Types and Utilities:

• Introduced OpenTelemetryDoubleHistogram, a wrapper for OpenTelemetry's double histogram metric, supporting recording with base and additional attributes.
• Added OpenTelemetryHelper, a utility class for handling OpenTelemetry attributes, including merging and converting maps to attributes.

Standardized Gobblin Metric Names and Dimensions:

• Added GobblinOpenTelemetryMetrics, an enum defining standard metric names and types for Gobblin job state and latency, with methods to create metrics.
• Added GobblinOpenTelemetryMetricsConstants, defining standard dimension keys and values for Gobblin job metrics (e.g., state transitions, job phases).

JIRA

• My PR addresses the following Gobblin JIRA issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
  • https://issues.apache.org/jira/browse/GOBBLIN-2209

Description

• Here are some details about my PR, including screenshots (if applicable):

• Added change to OpenTelemetryMetrics to emit ExponentialHistogramMetrics

• Created package org.apache.gobblin.metrics.opentelemetry; which contains different classes required for OTelMetrics emission

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:

• OpenTelemetryHelperTest

• OpenTelemetryInstrumentationTest

• OpenTelemetryMetricTest

Commits

• My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

[Copilot]

Pull Request Overview

This PR adds support for emitting OpenTelemetry metrics in Gobblin, including the integration of exponential histogram metrics and long counter metrics via newly introduced classes and activity types.

• Added new activity and helper classes to emit and manage OpenTelemetry metrics (e.g., EmitOTelMetrics, EmitOTelMetricsImpl).
• Updated workflows, workers, and job launchers to incorporate metric emission, and added unit tests for the new metrics functionality.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
CommitStepWorkflowImpl.java	Added metric emission using EmitOTelMetrics with new attribute assignments.
WorkFulfillmentWorker.java	Included EmitOTelMetricsImpl in the activity implementation array.
ExecuteGobblinJobLauncher.java	Integrated metric emission before and after workflow execution, including latency histograms.
EmitOTelMetricsImpl.java & EmitOTelMetrics.java	New activity interface and implementation for OpenTelemetry metric emission.
ActivityType.java & GobblinTemporalConfigurationKeys.java	Updated to define new activity type EMIT_OTEL_METRICS and configuration keys.
Various files under gobblin-metrics	Added support for OpenTelemetry metrics instrumentation, including counters, histograms, and tests.
ConfigurationKeys.java (gobblin-api)	Updated to include OpenTelemetry-related configuration keys.

Comments suppressed due to low confidence (1)

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/workflow/impl/CommitStepWorkflowImpl.java:51

• Consider replacing the literal 'currState' with the constant GaaSOpenTelemetryMetricsConstants.DimensionKeys.CURR_STATE for consistency with the rest of the codebase.

attributes.put("currState", "processWUStart");

[abhishekmjain]

1 is metric unit here?

[abhishekmjain]

Similarly s in the below one?

[Blazer-007]

1 is default unit here , although we should use curly annotation as mentioned in wiki but have kept default for simplicity
s represents second to measure latency metrics
https://opentelemetry.io/docs/specs/semconv/general/metrics/

[abhishekmjain]

Shall we use MethodUtils helper class instead of invoking methods ourself?

[Blazer-007]

We can do but this is the standard pattern being followed across codebase

https://github.com/search?q=repo%3Aapache%2Fgobblin+GobblinConstructorUtils.invokeLongestConstructor&type=code

gobblin/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/dataset/DatasetUtils.java

Line 92 in ddd9468

Class<?> datasetFinderClass = Class.forName(className);

[abhishekmjain]

description and unit can be used directly

[abhishekmjain]

can you please elaborate on why are we doing this?

[Blazer-007]

Nothing specific, just avoiding creating a new Map object

[khandelwal-prateek]

what's the difference between state and currState?

[Blazer-007]

State -- GenWU, ProcessWU, CommitStep
CurrState -- GenWUStart, GenWUComplete, ...

[khandelwal-prateek]

the metrics map uses only metricName as key. If two metrics share the same name but differ in type (eg, LONG_COUNTER vs DOUBLE_HISTOGRAM), this can silently cause incorrect caching behavior. It would be better to use a composite key like metricName + "_" + metricType to avoid collisions

[Blazer-007]

Two attributes, two metrics, or two events MUST NOT share the same name. Different entities (attribute and metric, metric and event) MAY share the same name.
https://opentelemetry.io/docs/specs/semconv/general/naming/#name-reuse-prohibition

[khandelwal-prateek]

We can move the metric creation logic into the GaaSOpenTelemetryMetrics enum itself instead of centralizing it in OpenTelemetryInstrumentation.createMetric.

Currently, the enum acts as a metadata holder, while the actual instantiation logic(via switch-case) is external and hence scattered. This becomes harder to extend if we introduce new metric types(eg gauges, timers, etc). A more extensible approach would be to let each enum constant hold a factory method and expose a createMetric() method

[khandelwal-prateek]

how are we using metricUnit? 1 is string here.. the metric is incremented directly on emission emitOTelMetrics.emitLongCounterMetric(GobblinOpenTelemetryMetrics.GOBBLIN_JOB_STATE, 1L, attributes, finalProps);

[khandelwal-prateek]

the workflow implementations is now directly dependent on OpenTelemetry which creates tight coupling with Otel, this makes it harder to switch metrics system in future. We can introduce a metrics abstraction and use Otel metrics as one implementation.

public interface JobMetricsRecorder {
...
}

public class OpenTelemetryJobMetricsRecorder implements JobMetricsRecorder {
    private final OpenTelemetryInstrumentation instrumentation;
    ...
}

public class ProcessWorkUnitsWorkflowImpl implements ProcessWorkUnitsWorkflow {
    private JobMetricsRecorder metricsRecorder;
    ...
}

[Blazer-007]

The current implementation is tight coupling wrt name only , we can look at the name change end to end later on if required as OpenTelemetry name is being used across orchestrator as well to emit final status metric.
In suggested interface & its impl we are just adding extra hop and the object of that impl can't be directly used inside temporal workflow as OpenTelemetryJobMetricsRecorder -> OpenTelemetryInstrumentation -> meter and meter object is not serializable so again we will need a temporal activity stub to connect to this JobMetricRecorder and that is what current impl is doing as well (if we ignore the name of class) , if we want to use other metric system in future we can just change EmitOTelMetricsImpl itself regardless of name.

I will raise a followup PR to address the name change of Activity interface and impl