-
Notifications
You must be signed in to change notification settings - Fork 102
Enhancement/allow custom metric buckets #781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
cretz
merged 22 commits into
temporalio:main
from
TheCodeWrangler:enhancement/allow-custom-metric-buckets
Apr 21, 2025
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
0e9666b
WIP
TheCodeWrangler b4d4706
WIP
TheCodeWrangler 3052733
Updated
TheCodeWrangler a7a318e
Removed format edits
TheCodeWrangler 97331d8
Removed format edits
TheCodeWrangler d760fb7
Removed format edits
TheCodeWrangler af83eea
Removed format edits
TheCodeWrangler 4b12391
Added a custom histogram
TheCodeWrangler b1ae028
Match prior linting
TheCodeWrangler ff0cb7a
Removed saving out metrics endpoint
TheCodeWrangler 5570148
Merge branch 'main' into enhancement/allow-custom-metric-buckets
cretz b9871bd
Merge branch 'main' into enhancement/allow-custom-metric-buckets
TheCodeWrangler e41206b
Updated after rebase and unit tests working on workflow endtoend metr…
TheCodeWrangler 4f3a1b4
Updated format with poe
TheCodeWrangler 7f5f0cd
Removed invalid comment
TheCodeWrangler b9a43e9
Updated to assert eventually on metrics check
TheCodeWrangler 319d758
Update tests/test_runtime.py
TheCodeWrangler 5313c1d
Work on test flake
cretz 7d2199f
Removed temporal_workflow_endtoend_latency from test
cretz dbb2e22
Merge branch 'main' into enhancement/allow-custom-metric-buckets
TheCodeWrangler 77a900e
CI flake investigations
cretz 04bc99e
Merge branch 'main' into enhancement/allow-custom-metric-buckets
cretz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for completeness sake, can you also add a check for custom metric? Basically just make a histogram override for your custom metric too, just assign
Runtime
to a var, and in addition to all that you're doing below, useruntime.metric_meter()
to create/record a custom histogram metric value and confirm it too gets the histogram override.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a
custom_histogram
and verified the buckets are updated.This work is actually NOT accomplishing what I want in the ability to control the binning of
temporal_workflow_endtoend_latency_bucket
andtemporal_activity_execution_latency_[milliseconds]_bucket
Are you able to tell if I will be able to do this in a PR on this repository or if it will require an update to
sdk-core
for that functionality?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may have to remove the
temporal_
prefix. If that is indeed the case, may be a good thing to document where the override attr is defined.