added custom metric sample #177
Conversation
|
This is failing on the black formatter, but I formatted and sorted the imports |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a custom metric sample to demonstrate how to record the workflow type in the activity schedule-to-start latency.
- Added a new workflow (ExecuteActivityWorkflow) and associated client code for executing activities.
- Introduced a custom activity interceptor to record custom metrics in the activity worker.
- Updated README files to include instructions for running the custom metric components.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| custom_metric/workflow_worker.py | Introduces a simple workflow that executes an activity. |
| custom_metric/client.py | Provides a client to start the new workflow. |
| custom_metric/activity_worker.py | Implements an interceptor for recording custom metrics for activity timing. |
| custom_metric/README.md | Updates sample usage instructions for the custom metric sample. |
| README.md | Adds the custom_metric sample to the main project documentation. |
custom_metric/activity_worker.py
Outdated
| ) | ||
|
|
||
|
|
||
| class SimpleWorkerInterceptor(Interceptor): |
There was a problem hiding this comment.
This sample is not just demonstrating custom metric, which can be done inside the activity, it's demonstrating interceptors. May be a little confusing to someone just wanting to understand the custom metric part.
There was a problem hiding this comment.
Thanks for the suggestion! I implemented it because what you mentioned off-PR, I thought you suggested to do this with an interceptor
There was a problem hiding this comment.
For this specific custom metric that makes sense, but people can create custom metrics without interceptors. I think it's fine to stay, may just need to clarify in README via that one/two-sentence explainer that the purpose of this sample is to demonstrate a custom schedule to start metric via interceptor, as opposed to just general custom metric.
There was a problem hiding this comment.
I added a little blurb. I hope it's along the lines you're thinking
There was a problem hiding this comment.
Can you add a test or two? (I know not all samples have tests, something we regret, but adding them helps us catch if they start breaking)
|
@cretz thanks for the review! I think all your comments have been acknowledged and fixed |
2 participants
What was changed
Added a custom metric sample
Why?
Help folks with questions about custom metrics
Checklist
How was this tested:
Checked the metric output against the existing metric
Any docs updates needed?
Updated main python samples readme