-
Notifications
You must be signed in to change notification settings - Fork 126
Camunda Pyzeebe Instrumentation #1385
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1385 +/- ##
==========================================
- Coverage 81.43% 81.40% -0.03%
==========================================
Files 210 211 +1
Lines 23875 23950 +75
Branches 3764 3782 +18
==========================================
+ Hits 19442 19497 +55
- Misses 3168 3177 +9
- Partials 1265 1276 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Posting a partial review-ran out of time for today but I'll review the tests tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this PR needs to be rebased onto the latest main. It looks like there's also some missing coverage so we need to add some more tests for the following:
- For what was previously the if-statements at the top of the new hooks file (now they are calling that extract function) we need to add coverage for message_id under publish_message and resources under deploy_resource.
- The background task creation else clause is missing coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this looks really good now! I had one note about he resourceCount getting dropped and the only other thing I see in coverage now is we need a test to cover this line:
https://app.codecov.io/github/newrelic/newrelic-python-agent/commit/93d1520084d254e15c030b895972c73290856e9a#c9c66516ae9ada6bf1c147f9dda06069-R93
Basically create a test without a background_task decorator and expect no data present. There are examples of this in other instrumentation tests you can use as a reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Looks like we need to rebase onto latest again before merging.
Sync from upstream/main
Merged upstream to fork - should be good now @hmstepanek ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 thanks for your collaboration on this PR!
* Enable coverage for motor instrumentation (#1487) * Drop CI Image Build Caching (#1493) * Add deprecation warning for modules (#1490) * Add deprecation warning for modules * Megalinter fixes * Bump github/codeql-action in the github_actions group (#1492) Bumps the github_actions group with 1 update: [github/codeql-action](https://github.com/github/codeql-action). Updates `github/codeql-action` from 3.30.1 to 3.30.3 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@f1f6e5f...192325c) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 3.30.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github_actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Add Additional Delay to Test Startup Fixture (#1494) * Add additional delay to test startup fixture * Fix assertion error message --------- Co-authored-by: Uma Annamalai <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Temporarily remove pypy310 test from pymysql and aiomysql (#1501) * Remove pymysql pypy310 test * Remove aiomysql pypy310 test * Falcon sanic testing fix (#1495) * Remove unsupported Sanic from tests * Remove unsupported Falcon from tests --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * Camunda Pyzeebe Instrumentation (#1385) * add initial pyzeebe instrumentation/tests * second attempt at tests (failing) * updated tests - 1/2 passing locally * combine function_trace client tests * fix: py agent team feedback #1 * fix: pyzeebe tests * chore: ruff formatting * fix: review updates #2 * fix: next round of updates * fix: more updates based on feedback * fix: resource parameter capture * chore: ruff lint fixes * fix: no txn tests;add resourceCount attr back --------- Co-authored-by: Uma Annamalai <[email protected]> * Format with ruff (#1505) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Timothy Pansino <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Uma Annamalai <[email protected]> Co-authored-by: Keagan Peet <[email protected]>
This PR adds support for the Camunda Pyzeebe Module
@worker.task
/@router.task
Need help with writing some tests for this instrumentation though