[GOBBLIN-2246] Make Orchestrator MDM metric dimesnion configurable #4164
[GOBBLIN-2246] Make Orchestrator MDM metric dimesnion configurable #4164adithya2754 wants to merge 3 commits intoapache:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds runtime-configurable dimensions support for the jobSucceeded OpenTelemetry metric in GaaS observability events. The changes enable orchestrators to control which dimensions are emitted through configuration while maintaining backward compatibility with baseline dimensions.
Changes:
- Introduced configurable dimension mapping via
metrics.reporting.opentelemetry.jobSucceeded.dimensionsMapallowing orchestrators to map OTel dimension keys to GaaSJobObservabilityEvent Avro fields - Added support for per-job extra dimensions from job properties with all-or-nothing enforcement based on
maxDimensionscap (default: 20) - Refactored tests to use try-with-resources pattern for proper resource cleanup
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| GaaSJobObservabilityEventProducer.java | Core implementation adding dimension configuration logic, baseline dimension preservation, duplicate detection, and dimension cap enforcement |
| GaaSJobObservabilityProducerTest.java | Updated existing tests to use try-with-resources; added 6 new tests covering dimension map configurations, baseline backfilling, duplicate handling, and maxDimensions enforcement |
| GsonUtils.java | Added plain GSON instance for general-purpose JSON parsing without special adapters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Blazer-007
changed the title
| if (value instanceof Number) { | ||
| builder.put(mdmDimensionKey, ((Number) value).longValue()); | ||
| return; | ||
| } |
There was a problem hiding this comment.
If we get a double, we will be sending a long value, is that intentional?
There was a problem hiding this comment.
While I agree a dimension usually would not have floating values, but its better to call that out somewhere.
|
|
||
| /** | ||
| * Reads the JSON string config for job succeeded dimensions map, which is expected to be in the format of | ||
| * {@code <mdm_dim_key>: <gaas_obs_event_field_key>}. |
There was a problem hiding this comment.
Can you check if this value can be easily passed from configs? Given that we haven't used such pattern yet, would be better to confirm once.
| return jobSucceededMaxDimensions; | ||
| } | ||
|
|
||
| private static Map<String, String> dropDuplicateObsEventFieldMappings(Map<String, String> configuredMap, |
There was a problem hiding this comment.
Is this really required?
2 participants
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
Updated
jobSucceededOTel metric emission to support runtime-configurable dimensions.metrics.reporting.opentelemetry.jobSucceeded.dimensionsMap(OTel key →GaaSJobObservabilityEventAvro field).metrics.reporting.opentelemetry.jobSucceeded.extraDimensions.enabledgate.metrics.reporting.opentelemetry.jobSucceeded.maxDimensions(default 20). If baseline+orchestrator exceeds the cap, orchestrator dims are dropped; template extras are included only if all fit.Tests
Added unit tests
Commits