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

[WIP] Wrapper script #1709

Merged
merged 32 commits into from
Jan 25, 2023
Merged

[WIP] Wrapper script #1709

merged 32 commits into from
Jan 25, 2023

Conversation

basepi
Copy link
Contributor

@basepi basepi commented Dec 12, 2022

What does this pull request do?

Adds the wrapper script to automate instrumentation without code changes.

Also adds support for no-code-changes instrumentation in Flask, plus a bunch of test fixes around that.

Related issues

Closes #1019
Closes #1598

@apmmachine
Copy link
Contributor

apmmachine commented Dec 12, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview previewSnapshots

Expand to view the summary

Build stats

  • Start Time: 2023-01-25T19:55:40.355+0000

  • Duration: 19 min 33 sec

Test stats 🧪

Test Results
Failed 0
Passed 5081
Skipped 3679
Total 8760

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /test linters : Run the Python linters only.

  • /test full : Run the full matrix of tests.

  • /test benchmark : Run the APM Agent Python benchmarks tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@apmmachine
Copy link
Contributor

apmmachine commented Dec 12, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (73/73) 💚
Files 100.0% (241/241) 💚
Classes 100.0% (241/241) 💚
Lines 91.927% (18754/20401) 👎 -0.146
Conditionals 75.046% (2884/3843) 👎 -0.118

@@ -163,6 +168,7 @@ markers =
aiobotocore
kafka
grpc
addopts=--random-order
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this while debugging the test failures on the old flask tests. I think we should leave it on, to try to prevent adding any tests that rely on order.

@@ -445,7 +445,7 @@ def test_backdating_transaction(elasticapm_client):
elasticapm_client.begin_transaction("test", start=time.time() - 1)
elasticapm_client.end_transaction()
transaction = elasticapm_client.events[constants.TRANSACTION][0]
assert 1000 < transaction["duration"] < 2000
assert 900 < transaction["duration"] < 2000
Copy link
Contributor Author

@basepi basepi Jan 12, 2023

Choose a reason for hiding this comment

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

This was a windows failure:

[2023-01-11T19:42:05.004Z] elasticapm_client = <tests.fixtures.TempStoreClient object at 0x0000018EBF6B4188>
[2023-01-11T19:42:05.004Z] 
[2023-01-11T19:42:05.004Z]     def test_backdating_transaction(elasticapm_client):
[2023-01-11T19:42:05.004Z]         elasticapm_client.begin_transaction("test", start=time.time() - 1)
[2023-01-11T19:42:05.004Z]         elasticapm_client.end_transaction()
[2023-01-11T19:42:05.004Z]         transaction = elasticapm_client.events[constants.TRANSACTION][0]
[2023-01-11T19:42:05.004Z] >       assert 1000 < transaction["duration"] < 2000
[2023-01-11T19:42:05.004Z] E       assert 1000 < 999.971
[2023-01-11T19:42:05.004Z] 
[2023-01-11T19:42:05.004Z] tests\client\transaction_tests.py:448: AssertionError

I assume it's a side effect of --random-order....or maybe it's a clock issue on the VM? I don't know. I just added some wiggle room.

@basepi basepi marked this pull request as ready for review January 13, 2023 17:25
@basepi basepi requested a review from beniwohli January 13, 2023 17:25
Copy link
Contributor

@beniwohli beniwohli left a comment

Choose a reason for hiding this comment

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

Great work!

docs/wrapper.asciidoc Outdated Show resolved Hide resolved
elasticapm/instrumentation/register.py Outdated Show resolved Hide resolved
docs/wrapper.asciidoc Outdated Show resolved Hide resolved
@basepi basepi requested a review from beniwohli January 18, 2023 19:17
Copy link
Contributor

@beniwohli beniwohli left a comment

Choose a reason for hiding this comment

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

Just one little nitpick, otherwise 👍 . Great work!

elasticapm/instrumentation/register.py Outdated Show resolved Hide resolved
@basepi basepi enabled auto-merge (squash) January 25, 2023 18:04
@basepi basepi merged commit 7b2de88 into elastic:main Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No-Code-Changes Instrumentation: Flask Wrapper script for no-code-changes onboarding
3 participants