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.

Sign up for GitHub

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

Jump to bottom

GUNDI-2876: Refactor ER-SMART to use pubsub #287

Merged
merged 9 commits into from
Apr 15, 2024
Merged

GUNDI-2876: Refactor ER-SMART to use pubsub #287

merged 9 commits into from
Apr 15, 2024

Conversation

marianobrc
Copy link
Contributor

@marianobrc marianobrc commented Apr 12, 2024 •
edited
Loading

What does this PR do?

  • Changes the ER->SMART integration to publish events and patrol in PubSub topics instead of Kafka topics
  • Adds basic test coverage for the ER->SMART integration for events and patrols
  • Improves OTel instrumentation so we can trace events and patrols extracted by the ER->SMART integration

Relevant link(s)

GUNDI-2876

@marianobrc marianobrc force-pushed the gundi-2876-ersmart-pubsub branch from ebac9cb to d2e8713 Compare April 12, 2024 22:48
chrisdoehring
chrisdoehring reviewed Apr 13, 2024
View reviewed changes
cdip_admin/core/tracing/instrumentation.py Outdated
Comment on lines 53 to 54
span.set_attribute("patrols", str(event.patrols))
span.set_attribute("patrols", str(event.files))
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like an attribute name collision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this, will fix it.

chrisdoehring
chrisdoehring approved these changes Apr 13, 2024
View reviewed changes
Copy link
Contributor

@chrisdoehring chrisdoehring left a comment

Choose a reason for hiding this comment

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

I just found one small think in the span attributes. Aside from that it looks perfect.

@marianobrc marianobrc merged commit 501f2ac into main Apr 15, 2024
1 check passed
@marianobrc marianobrc deleted the gundi-2876-ersmart-pubsub branch April 15, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisdoehring chrisdoehring chrisdoehring approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@marianobrc @chrisdoehring