[Instrumentation.Hangfire] Add metrics support#3258
Conversation
289b75d to
5655113
Compare
thompson-tomo
left a comment
There was a problem hiding this comment.
Looks to be a good step forward and I have suggested a couple of changes here as well as to the sem conv based on the learnings.
|
@gorbach, please check this comment #2075 (comment) The second point is technical requirements to separate metrics/traces signal. It will be great to rename classes in the separate PR and then merge it/rebase here. |
Implements OpenTelemetry workflow semantic conventions for Hangfire metrics. Fixes open-telemetry#2075
…ve trigger.type to workflow-level metrics, add workflow.count
… DisplayNameFunc option
dd6e74a to
d43ee2c
Compare
thompson-tomo
left a comment
There was a problem hiding this comment.
Looking good. Only 1 design question #3258 (comment) which when resolved would mean I approve.
thompson-tomo
left a comment
There was a problem hiding this comment.
Have left a bunch of nits for things which could be improved/simplified.
…fireMetricsErrorFilterAttribute.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireMetricsInstrumentation.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireMetricsStateFilter.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireMetricsStateFilter.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireTagBuilder.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireMetricsStateFilter.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireMetricsErrorFilterAttribute.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireTagBuilder.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
…fireMetricsStateFilter.cs Co-authored-by: James Thompson <thompson.tomo@outlook.com>
|
I beleive I've fixed all linter warnings, compilation errors (in release) and ran |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3258 +/- ##
==========================================
+ Coverage 70.80% 71.21% +0.40%
==========================================
Files 440 442 +2
Lines 17265 17516 +251
==========================================
+ Hits 12225 12474 +249
- Misses 5040 5042 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
thompson-tomo
left a comment
There was a problem hiding this comment.
Note these changes would be necessary once #3545 is merged so might be better to merge them now. Could a maintainer commit my suggestions?
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
261c8c5 to
aa2d422
Compare
|
Hi, when's this hitting NuGet? Thanks! |
|
Someone needs to create https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/new?template=release_request.yml. Probably 1.14.0-beta.2 version. |
Fixes #2075
Changes
Adds OpenTelemetry metrics support to Hangfire instrumentation following workflow semantic conventions.
Implemented metrics:
workflow.execution.count- Counter for task executionsworkflow.execution.duration- Histogram for execution durationworkflow.execution.status- UpDownCounter for state transitionsworkflow.execution.errors- Counter for execution errorshangfire.queue.latency- Optional histogram for queue wait timeMerge requirement checklist