Skip to content

Manually instrument traces in pixl_cli, orthanc_raw, and orthanc_anon#644

Merged
p-j-smith merged 12 commits into
mainfrom
paul/638-manually-instrument-traces
Jun 29, 2026
Merged

Manually instrument traces in pixl_cli, orthanc_raw, and orthanc_anon#644
p-j-smith merged 12 commits into
mainfrom
paul/638-manually-instrument-traces

Conversation

@p-j-smith

@p-j-smith p-j-smith commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #639 Instrument the cli, Orthanc Raw, and Orthanc Anon so we can follow a DICOM study end-to-end

  • add a new function pixl_core.tracing.configure_tracing to set up a tracing provider
  • instrument the CLI. We first need to set relevant environment variables, which is done using _configure_telemetry_env_vars. Then configure tracing and instrument pika
  • manually start a span before each message to the queue
  • instrument Orthanc Raw. This is currently only instrumented for record_dicom_headers, not for the actual C-MOVE
  • instrument Orthanc Anon
  • add tests to check logs and traces are correlated
  • move the fixtures created for testing logging into the conftest so they can be reused for testing tracing

Type of change

Please delete options accordingly to the description.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Suggested Checklist

  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have passed on my local host device. (see further details at the CONTRIBUTING document)
  • Make sure your branch is up-to-date with main branch. See CONTRIBUTING for a general example to syncronise your branch with the main branch.
  • I have requested review to this PR.
  • I have addressed and marked as resolved all the review comments in my PR.
  • Finally, I have selected squash and merge

All logs selected for a given trace_id (the trace corresponds to processing one of the DICOM studies in the integration tests):

logs.mov

All spans for the same trace_id

trace.mov

@p-j-smith p-j-smith requested a review from stefpiatek June 18, 2026 13:13
@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.18182% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.53%. Comparing base (fe104ac) to head (1109433).

Files with missing lines Patch % Lines
cli/src/pixl_cli/main.py 91.66% 1 Missing ⚠️
pixl_core/src/core/patient_queue/producer.py 92.85% 1 Missing ⚠️
pixl_core/src/core/tracing.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #644      +/-   ##
==========================================
+ Coverage   85.85%   88.53%   +2.68%     
==========================================
  Files          75       80       +5     
  Lines        3500     3813     +313     
==========================================
+ Hits         3005     3376     +371     
+ Misses        495      437      -58     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stefpiatek stefpiatek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎉

Comment thread pixl_core/src/core/tracing.py
Comment thread pixl_core/tests/conftest.py
Comment thread docker-compose.yml
@p-j-smith p-j-smith merged commit f4ad704 into main Jun 29, 2026
11 checks passed
@p-j-smith p-j-smith deleted the paul/638-manually-instrument-traces branch June 29, 2026 15:36
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.

add manual OTel instrumentation where needed

2 participants