Skip to content

Generate OTel metrics with context limits#1331

Merged
blt merged 5 commits intomainfrom
blt/generate_otel_metrics_with_context_limits
May 15, 2025
Merged

Generate OTel metrics with context limits#1331
blt merged 5 commits intomainfrom
blt/generate_otel_metrics_with_context_limits

Conversation

@blt
Copy link
Copy Markdown
Collaborator

@blt blt commented Apr 24, 2025

What does this PR do?

Generate OTel metrics with context limits

This commit allows users to configure an otel metrics payload that
has a fixed number of 'contexts' to reuse the term from dogstatsd.
This is done by setting a lot of fiddly knobs and the user is
responsible for keeping track of how many unique instances are
possible.

This commit will be chased with some clean-up as I left a lot of
internal duplication in the code. I am also unsure if this generates
sensible load run against a collector, just that it obeys the
protocol by construction.

REF SMPTNG-659

Copy link
Copy Markdown
Collaborator Author

blt commented Apr 24, 2025

@blt blt added the no-changelog label Apr 24, 2025 — with Graphite App
@blt blt marked this pull request as ready for review April 24, 2025 21:36
@blt blt requested a review from a team as a code owner April 24, 2025 21:36
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from 089e4d7 to 74cc380 Compare April 28, 2025 19:05
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from 74cc380 to eae62f5 Compare April 28, 2025 19:24
@blt blt force-pushed the blt/otel_metrics_common_dogstatsd branch from b17ae82 to 5d2d1c0 Compare April 28, 2025 19:24
@blt blt force-pushed the blt/otel_metrics_common_dogstatsd branch from 5d2d1c0 to b01874b Compare April 29, 2025 17:02
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from 19d131a to 6a90429 Compare April 29, 2025 17:02
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from 6a90429 to f18df04 Compare May 8, 2025 18:46
@blt blt force-pushed the blt/otel_metrics_common_dogstatsd branch from b01874b to aaef818 Compare May 8, 2025 18:46
@blt blt force-pushed the blt/otel_metrics_common_dogstatsd branch from aaef818 to 9ffbdec Compare May 8, 2025 19:02
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from f18df04 to 6601f83 Compare May 8, 2025 19:02
@blt blt force-pushed the blt/otel_metrics_common_dogstatsd branch from 9ffbdec to 9d8800f Compare May 8, 2025 19:14
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from 6601f83 to 6d042b6 Compare May 8, 2025 19:14
@blt blt force-pushed the blt/otel_metrics_common_dogstatsd branch from 9d8800f to bb9ab4d Compare May 12, 2025 18:02
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from 6d042b6 to 10f6fca Compare May 12, 2025 18:02
@blt blt force-pushed the blt/convert_opentelemetry_metric_to_use_membergenerator_method branch 2 times, most recently from 7a78bb2 to 7bfe432 Compare May 14, 2025 16:52
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch 2 times, most recently from 2687544 to da8aa60 Compare May 14, 2025 17:02
@blt blt force-pushed the blt/convert_opentelemetry_metric_to_use_membergenerator_method branch from 7bfe432 to d6f15cb Compare May 14, 2025 21:04
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from da8aa60 to 9293145 Compare May 14, 2025 21:04
@blt blt changed the base branch from blt/convert_opentelemetry_metric_to_use_membergenerator_method to graphite-base/1331 May 14, 2025 21:48
blt added 4 commits May 14, 2025 21:58
This commit allows users to configure an otel metrics payload that
has a fixed number of 'contexts' to reuse the term from dogstatsd.
This is done by setting a lot of fiddly knobs and the user is
responsible for keeping track of how many unique instances are
possible.

This commit will be chased with some clean-up as I left a lot of
internal duplication in the code. I am also unsure if this generates
sensible load run against a collector, just that it obeys the
protocol by construction.

REF SMPTNG-659

Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
This commit avoids non-trivial duplication of the same string pool allocation.

Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from 9293145 to ee13a5a Compare May 14, 2025 21:58
@blt blt force-pushed the graphite-base/1331 branch from d6f15cb to d4f3c65 Compare May 14, 2025 21:58
@graphite-app graphite-app bot changed the base branch from graphite-base/1331 to main May 14, 2025 21:59
Signed-off-by: Brian L. Troutwine <brian.troutwine@datadoghq.com>
@blt blt force-pushed the blt/generate_otel_metrics_with_context_limits branch from ee13a5a to db2e90d Compare May 14, 2025 21:59
@dd-devflow
Copy link
Copy Markdown

dd-devflow bot commented May 14, 2025

View all feedbacks in Devflow UI.

2025-05-14 22:44:05 UTC ℹ️ Start processing command /merge


2025-05-14 22:44:09 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.


2025-05-15 02:45:04 UTC ⚠️ MergeQueue: This merge request was unqueued

devflow unqueued this merge request: It did not become mergeable within the expected time

None
};

let total_scopes = self.scopes_per_resource.sample(rng);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if resource is None, then we still generate scope metrics. Is this intentional? The naming of scopes_per_resource makes me question if this is correct (ie, if we have a no resource, then there should be no scopes)

Ok(v1::ScopeMetrics {
scope: Some(instrumentation_scope),
metrics,
schema_url: String::new(), // 0-sized alloc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is a zero-sized alloc? Asking more from a curiosity of how rust handles this

Copy link
Copy Markdown
Contributor

@scottopell scottopell left a comment

Choose a reason for hiding this comment

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

looks good overall, a few thoughts in-line, but approving as they may be addressed upstack

@blt blt merged commit 0c990f7 into main May 15, 2025
24 of 25 checks passed
Copy link
Copy Markdown
Collaborator Author

blt commented May 15, 2025

Merge activity

  • May 15, 11:45 AM EDT: @blt merged this pull request with Graphite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants