Skip to content

Bound contexts in Opentelemetry Metrics#1337

Merged
blt merged 12 commits intomainfrom
blt/bound_contexts_in_opentelemetry_metrics
May 15, 2025
Merged

Bound contexts in Opentelemetry Metrics#1337
blt merged 12 commits intomainfrom
blt/bound_contexts_in_opentelemetry_metrics

Conversation

@blt
Copy link
Copy Markdown
Collaborator

@blt blt commented Apr 30, 2025 •
edited
Loading

What does this PR do?

Bound contexts in OpenTelemetry metrics generation by use of template
mechanism. The implementation here is slow -- note we are not yet using
the tag generator from down-stack, that will be addressed -- but it does
pass its property tests.

Motivation

REF SMPTNG-659

This was referenced Apr 30, 2025
Merged
Merged
Merged
@blt blt added the no-changelog label Apr 30, 2025 — with Graphite App
@blt blt mentioned this pull request Apr 30, 2025
Merged
@blt Graphite App
Copy link
Copy Markdown
Collaborator Author

blt commented Apr 30, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@blt blt mentioned this pull request Apr 30, 2025
Merged
@blt blt marked this pull request as ready for review April 30, 2025 21:37
@blt blt requested a review from a team as a code owner April 30, 2025 21:37
This was referenced Apr 30, 2025
Merged
Merged
scottopell
scottopell reviewed May 1, 2025
View reviewed changes
lading_payload/src/opentelemetry_metric/templates.rs
_ => unreachable!(),
};

Ndp(NumberDataPoint {
Copy link
Copy Markdown
Contributor

@scottopell scottopell May 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be worthwhile to include a comment referencing the upstream NumberDataPoint docs

Copy link
Copy Markdown
Collaborator Author

@blt blt May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add some commentary around this. The main module links to the data-model docs and the spec but the protobuf definition -- which is where I sourced information about this piece -- has further details yet. It's kind of spread around.

Copy link
Copy Markdown
Contributor

@scottopell scottopell May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this done up-stack? I don't see anything additional here

Copy link
Copy Markdown
Collaborator Author

@blt blt May 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added what I have on the subject in the comment block beginning line 41 in this PR. It's meager.

This was referenced May 1, 2025
Closed
Merged
@blt blt force-pushed the blt/make_tagset_generation_generic branch from 6d5ccc7 to d98f9e7 Compare May 8, 2025 18:46
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from 972f1c9 to afd9721 Compare May 8, 2025 18:46
This was referenced May 8, 2025
Closed
Closed
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from afd9721 to ff7beef Compare May 8, 2025 19:02
@blt blt force-pushed the blt/make_tagset_generation_generic branch 2 times, most recently from d828cd2 to 832c121 Compare May 8, 2025 19:14
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from ff7beef to 1d5cdf2 Compare May 8, 2025 19:14
@blt blt force-pushed the blt/make_tagset_generation_generic branch from 832c121 to 5e0b3b6 Compare May 12, 2025 18:02
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from 1d5cdf2 to 6f427f7 Compare May 12, 2025 18:02
This was referenced May 12, 2025
Merged
Merged
@blt blt force-pushed the blt/make_tagset_generation_generic branch from 5e0b3b6 to c002711 Compare May 12, 2025 21:43
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from 6f427f7 to 02e8aa4 Compare May 12, 2025 21:43
@datadog-datadog-prod-us1
Copy link
Copy Markdown

datadog-datadog-prod-us1 bot commented May 12, 2025 •
edited
Loading

Datadog Summary

✅ Code Quality    ✅ Code Security    ✅ Dependencies

Was this helpful? Give us feedback!

@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from 02e8aa4 to 4bd6e06 Compare May 12, 2025 21:46
@blt blt force-pushed the blt/generate_otel_units branch from 1b58fc0 to c9d262d Compare May 14, 2025 22:05
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from 0966a3e to 922e878 Compare May 14, 2025 22:05
@blt blt force-pushed the blt/generate_otel_units branch 2 times, most recently from 198b629 to 7505fde Compare May 15, 2025 15:46
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from 922e878 to c0e8cb9 Compare May 15, 2025 15:46
@blt blt changed the base branch from blt/generate_otel_units to graphite-base/1337 May 15, 2025 15:57
blt added 11 commits May 15, 2025 15:57
@blt
Bound contexts in Opentelemetry Metrics
be30525 
This commit asserts that contexts are properly bounded in OTel metrics generation.

Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint
590cdd7 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint
8863ba6 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint
62bc5ff 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
triggered failure, also remembered that prop_assert is a nicer debug …
b0b6a97 
…experiment

Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint
799837f 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint
2cf7f97 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint
17bbd4c 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint
817192e 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
checkpoint before tests
dc6bc83 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt
property tests pass, contexts bounded
aeeed2e 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from c0e8cb9 to ff661c3 Compare May 15, 2025 15:58
@blt blt force-pushed the graphite-base/1337 branch from 7505fde to 43e2b56 Compare May 15, 2025 15:58
@graphite-app graphite-app bot changed the base branch from graphite-base/1337 to main May 15, 2025 15:58
@blt
PR review
de3a40b 
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt blt force-pushed the blt/bound_contexts_in_opentelemetry_metrics branch from ff661c3 to de3a40b Compare May 15, 2025 15:58
@blt blt merged commit fd420d0 into main May 15, 2025
21 checks passed
@blt Graphite App
Copy link
Copy Markdown
Collaborator Author

blt commented May 15, 2025

Merge activity

  • May 15, 1:24 PM EDT: @blt merged this pull request with Graphite.

@blt blt deleted the blt/bound_contexts_in_opentelemetry_metrics branch May 15, 2025 17:24
This was referenced May 15, 2025
Merged
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scottopell scottopell scottopell approved these changes

Assignees

No one assigned

Labels

no-changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@blt @scottopell