Skip to content
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

[Instrumentation.Hangfire] Add option to record exceptions #719

Merged
merged 11 commits into from
Oct 21, 2022
Merged

[Instrumentation.Hangfire] Add option to record exceptions #719

merged 11 commits into from
Oct 21, 2022

Conversation

vitor-pinto-maersk
Copy link
Contributor

@vitor-pinto-maersk vitor-pinto-maersk commented Oct 20, 2022

Changes

Adds support to optionally record hangfire job exceptions.

@vitor-pinto-maersk vitor-pinto-maersk requested a review from a team October 20, 2022 10:26
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 20, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@vitor-pinto-maersk vitor-pinto-maersk changed the title Add option to record hangfire exceptions [Instrumentation.Hangfire] Add option to record exceptions Oct 20, 2022
Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Changes looks good to me.
Please also add changelog entry

@cijothomas
Copy link
Member

@fred2u Could you review?

@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Merging #719 (18981df) into main (3a9e248) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 18981df differs from pull request most recent head 3df804d. Consider uploading reports for the commit 3df804d to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #719      +/-   ##
==========================================
- Coverage   77.70%   77.65%   -0.05%     
==========================================
  Files         175      176       +1     
  Lines        5333     5335       +2     
==========================================
- Hits         4144     4143       -1     
- Misses       1189     1192       +3     
Impacted Files Coverage Δ
...ntation.Hangfire/HangfireInstrumentationOptions.cs 100.00% <100.00%> (ø)
...ation/HangfireInstrumentationJobFilterAttribute.cs 86.36% <100.00%> (+2.57%) ⬆️
...tation.Hangfire/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...orter.Geneva/MsgPackExporter/MsgPackLogExporter.cs 88.55% <0.00%> (-5.51%) ⬇️
...ry.Exporter.Geneva/Metrics/GenevaMetricExporter.cs 61.00% <0.00%> (+2.90%) ⬆️

Copy link
Contributor

@utpilla utpilla left a comment

Choose a reason for hiding this comment

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

Consider enabling public API analyzer as well. This could be done in a follow-up PR.

@utpilla utpilla added the comp:instrumentation.hangfire Things related to OpenTelemetry.Instrumentation.Hangfire label Oct 20, 2022
@cijothomas cijothomas merged commit d21d3ca into open-telemetry:main Oct 21, 2022
@vitor-pinto-maersk vitor-pinto-maersk deleted the task/hangfire-record-exception branch October 24, 2022 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.hangfire Things related to OpenTelemetry.Instrumentation.Hangfire
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants