Skip to content

Use Correlation ID#103

Merged
callumforrester merged 3 commits intomainfrom
correlation-id
Mar 9, 2023
Merged

Use Correlation ID#103
callumforrester merged 3 commits intomainfrom
correlation-id

Conversation

@callumforrester
Copy link
Copy Markdown
Contributor

@callumforrester callumforrester commented Mar 8, 2023 •
edited
Loading

Changes:

  • Optionally allow correlation IDs to be propagated through internal events
  • Add optional correlation ID to context of received messages, derived from correlation-id header in stomp frame
  • Add optional correlation ID argument to send message method
  • Add tests for the above
  • Propagate correlation ID through worker events
  • Use correlation ID for plan requests or generate a new one if not present

@callumforrester callumforrester requested a review from a user March 8, 2023 16:17
@callumforrester callumforrester added the enhancement New feature or request label Mar 8, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2023 •
edited
Loading

Codecov Report

Merging #103 (ed47a77) into main (79b0477) will decrease coverage by 0.26%.
The diff coverage is 52.17%.

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
- Coverage   72.38%   72.13%   -0.26%     
==========================================
  Files          35       35              
  Lines         967      976       +9     
==========================================
+ Hits          700      704       +4     
- Misses        267      272       +5
Impacted Files Coverage Δ
src/blueapi/service/app.py 40.35% <0.00%> (ø)
src/blueapi/worker/reworker.py 33.33% <0.00%> (-1.35%) ⬇️
src/blueapi/core/event.py 100.00% <100.00%> (ø)
src/blueapi/messaging/base.py 100.00% <100.00%> (ø)
src/blueapi/messaging/context.py 100.00% <100.00%> (ø)
src/blueapi/messaging/stomptemplate.py 92.30% <100.00%> (+0.20%) ⬆️
src/blueapi/worker/task.py 69.76% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@callumforrester callumforrester marked this pull request as ready for review March 9, 2023 09:31
ghost
ghost reviewed Mar 9, 2023
View reviewed changes
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Works for correlating the request between services, don't know if you want to expose the id in the logs to allow you to close #96?

@callumforrester callumforrester merged commit 8e754f3 into main Mar 9, 2023
@callumforrester callumforrester deleted the correlation-id branch March 9, 2023 10:01
keithralphs pushed a commit that referenced this pull request Apr 28, 2023
@callumforrester @keithralphs
Use Correlation ID (#103)
528836c 
* Attach correlation ID to message headers of all messages associated with a particular task

* Fix correlation ID header name

* Fix tests and test correlation ID in stomp template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@callumforrester