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

Add opentelemetry integration #467

Closed
wants to merge 1 commit into from

Conversation

johnchildren
Copy link

@johnchildren johnchildren commented Jul 3, 2024

Add a draft opentelemetry integration consisting of a span propagator, a decorator for instrumenting jobs and a class for hooking into enqueue_job with span propagation. Additionally there are some metrics for the number of enqueued jobs.

This is a reasonably bare-bones implementation that needs work in a few areas which I will can come back to, but I think there are a few weaknesses I can think of:

  • Having to explicitly instrument jobs using the decorator is a bit clunky and perhaps it should happen automatically?
  • InstrumentedArqRedis is a bit awkward to use and differs quite a lot from the opentelemetry-contrib modules which use Instrumentors.
  • I'm not certain about whether this should be an extra to the main arq package or a separate extension.

edit: Obviously this also needs me to write some tests and docs...


I should also mention that this is adapted from our internal code base at Quantinuum, where both @isobelhooper and myself have been working on this intermittently over the past few years.

Add a draft opentelemtry integration consisting of a span propagator, a
decorator for instrumenting jobs and a class for hooking into
enqueue_job with span propagation. Additionally there are some metrics
for the number of enqueued jobs.

This is a reasonably bare-bones implementation that needs work in a few
areas which I will can come back to, but I think there are a few
weaknesses I can think of:

* Having to explicitly instrument jobs using the decorator is a bit
  clunky and perhaps it should happen automatically?
* `InstrumentedArqRedis` is a bit awkward to use and differs quite a lot
  from the `opentelemetry-contrib` modules which use Instrumentors.
* I'm not certain about whether this should be an extra to the main
  `arq` package or a separate extension.
@johnchildren johnchildren marked this pull request as draft July 3, 2024 14:42
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 0% with 164 lines in your changes missing coverage. Please review.

Project coverage is 83.73%. Comparing base (94cd878) to head (4c6b844).
Report is 11 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #467       +/-   ##
===========================================
- Coverage   96.27%   83.73%   -12.54%     
===========================================
  Files          11       16        +5     
  Lines        1074     1242      +168     
  Branches      209      211        +2     
===========================================
+ Hits         1034     1040        +6     
- Misses         19      182      +163     
+ Partials       21       20        -1     
Files Coverage Δ
arq/opentelemetry/shared.py 0.00% <0.00%> (ø)
arq/opentelemetry/__init__.py 0.00% <0.00%> (ø)
arq/opentelemetry/propagator.py 0.00% <0.00%> (ø)
arq/opentelemetry/consume.py 0.00% <0.00%> (ø)
arq/opentelemetry/produce.py 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1315583...4c6b844. Read the comment docs.

@quantinuum-dev quantinuum-dev closed this by deleting the head repository Nov 18, 2024
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 164 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
arq/opentelemetry/produce.py 0.00% 62 Missing and 1 partial ⚠️
arq/opentelemetry/consume.py 0.00% 48 Missing ⚠️
arq/opentelemetry/propagator.py 0.00% 46 Missing ⚠️
arq/opentelemetry/__init__.py 0.00% 4 Missing ⚠️
arq/opentelemetry/shared.py 0.00% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##             main     #467       +/-   ##
===========================================
- Coverage   96.27%   83.73%   -12.54%     
===========================================
  Files          11       16        +5     
  Lines        1074     1242      +168     
  Branches      209      211        +2     
===========================================
+ Hits         1034     1040        +6     
- Misses         19      182      +163     
+ Partials       21       20        -1     
Files with missing lines Coverage Δ
arq/opentelemetry/shared.py 0.00% <0.00%> (ø)
arq/opentelemetry/__init__.py 0.00% <0.00%> (ø)
arq/opentelemetry/propagator.py 0.00% <0.00%> (ø)
arq/opentelemetry/consume.py 0.00% <0.00%> (ø)
arq/opentelemetry/produce.py 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1315583...4c6b844. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants