Skip to content

refactor(tracking): replace flat operationType with dimensional attributes#623

Merged
jphui merged 2 commits into
masterfrom
jhui/otel-dao-benchmark-metrics
May 18, 2026
Merged

refactor(tracking): replace flat operationType with dimensional attributes#623
jphui merged 2 commits into
masterfrom
jhui/otel-dao-benchmark-metrics

Conversation

@jphui
Copy link
Copy Markdown
Contributor

@jphui jphui commented May 13, 2026

Summary

  • Rewrite BaseDaoBenchmarkMetrics so impls receive each dimension (operation, aspect, count bucket, status, error class) as a separate parameter instead of one concatenated operationType string.
  • Update InstrumentedEbeanLocalAccess to pass dimensions through cleanly: aspect class name for per-aspect ops, a bucketed count label (1..9, 10+) for batch ops.
  • Bucketing keeps cardinality bounded for downstream backends (OTEL especially).

Why

Today the impl gets strings like add.AliasesInfo, create.aspects_3, batchGetUnion.keys_42 — each becomes its own metric path in RRD/Dropwizard, so aggregating "all adds across all aspects" requires summing hundreds of named series. Splitting into native dimensions lets backends emit one metric with attributes and aggregate/filter natively. This is a pre-req for a downstream OTEL-backed impl.

Follow-up (required in metadata-graph-assets)

This PR changes the interface contract; it does not itself emit any metrics. The existing DropwizardDaoBenchmarkMetrics in metadata-graph-assets (the only known concrete impl) implements the old recordOperationLatency / recordOperationError methods and will not compile against the new contract.

When MGA picks up this version (via a metadata-models bump), it must in the same PR:

  1. Replace DropwizardDaoBenchmarkMetrics with a new DaoBenchmarkOtelMetrics that implements the single recordOperation(...) method.
    • Mirror the existing MgaClientMetrics facade pattern (typed AttributeKeys, pre-built LongCounter + LongHistogram, Meter obtained from com.linkedin.opentelemetry.OpenTelemetry.meterBuilder(...)).
    • Suggested metric names: Dao.Benchmark.Operation.Count (counter), Dao.Benchmark.Operation.Latency (histogram, ms).
    • Attribute keys (PascalCase, per LinkedIn MDM convention): Operation, EntityType, Aspect, CountBucket, Status, ErrorClass.
  2. Update DaoBenchmarkMetricsFactory.createInstance(...) to return the OTEL impl instead of the Dropwizard one.
  3. Delete DropwizardDaoBenchmarkMetrics and its DropwizardDaoBenchmarkMetricsTest.
  4. Add unit tests using InMemoryMetricReader (mirror MgaClientMetricsTest).

The MGA-side OTEL impl is where the real metric emission lives — this PR is purely kernel plumbing.

Test plan

  • :dao-api + :dao-impl:ebean-dao compile clean (Java 11)
  • InstrumentedEbeanLocalAccessTest passes — rewrote assertions to verify the 7-param contract, added bucketCount boundary tests (0, 1, 5, 9, 10, 100), covered success + failure paths
  • Full :dao-impl:ebean-dao:test suite (CI will run)

🤖 Generated with Claude Code

jphui added 2 commits May 13, 2026 03:40
…butes

Rewrite BaseDaoBenchmarkMetrics so implementations receive each dimension
(operation, aspect, count bucket, status, error class) as a separate
parameter instead of a single concatenated operationType string. Lets a
backend emit one counter/histogram with attributes rather than a separate
named metric per aspect or batch size.

Update InstrumentedEbeanLocalAccess to pass dimensions through cleanly:
aspect class name for per-aspect ops, a bucketed count label (1..9, 10+)
for batch ops. Bucketing keeps cardinality bounded for downstream backends.
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.86%. Comparing base (b025046) to head (26f992b).

Files with missing lines Patch % Lines
...din/metadata/dao/InstrumentedEbeanLocalAccess.java 80.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #623      +/-   ##
============================================
+ Coverage     66.82%   66.86%   +0.03%     
- Complexity     1880     1885       +5     
============================================
  Files           148      148              
  Lines          7262     7273      +11     
  Branches        879      880       +1     
============================================
+ Hits           4853     4863      +10     
- Misses         2024     2025       +1     
  Partials        385      385              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jphui jphui merged commit e528785 into master May 18, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants