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

implement transaction_ignore_urls #923

Merged
merged 28 commits into from
Nov 17, 2020

Conversation

beniwohli
Copy link
Contributor

What does this pull request do?

Building on #773, implement transaction_ignore_urls

Related issues

closes #772

@apmmachine
Copy link
Contributor

apmmachine commented Sep 15, 2020

💚 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

Expand to view the summary

Build stats

  • Build Cause: [Pull request #923 updated]

  • Start Time: 2020-11-16T16:05:00.875+0000

  • Duration: 24 min 15 sec

Test stats 🧪

Test Results
Failed 0
Passed 12094
Skipped 8375
Total 20469

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 12094
Skipped 8375
Total 20469

Copy link
Contributor

@basepi basepi left a comment

Choose a reason for hiding this comment

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

Looks great! Excellent testing coverage! 👍

@@ -345,6 +345,8 @@ class Config(_ConfigBase):
instrument_django_middleware = _BoolConfigValue("INSTRUMENT_DJANGO_MIDDLEWARE", default=True)
autoinsert_django_middleware = _BoolConfigValue("AUTOINSERT_DJANGO_MIDDLEWARE", default=True)
transactions_ignore_patterns = _ListConfigValue("TRANSACTIONS_IGNORE_PATTERNS", default=[])
transaction_ignore_urls = _ListConfigValue("TRANSACTION_IGNORE_URLS", type=starmatch_to_regex, default=[])

Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason we have an extra line here?

@@ -108,7 +108,10 @@ def get_current_url(environ, root_only=False, strip_querystring=False, host_only
:param strip_querystring: set to `True` if you don't want the querystring.
:param host_only: set to `True` if the host URL should be returned.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add path_only to the docstring?

import pytest


def pytest_generate_tests(metafunc):
Copy link
Contributor

Choose a reason for hiding this comment

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

Learned something new today! Didn't know about this pytest feature. 👍

@apmmachine
Copy link
Contributor

apmmachine commented Oct 27, 2020

🐛 Flaky test report

❕ There are test failures but not known flaky tests.

Expand to view the summary

Test stats 🧪

Test Results
Failed 1
Passed 12693
Skipped 8409
Total 21103

Genuine test errors 1

💔 There are test failures but not known flaky tests, most likely a genuine test failure.

  • Name: Initializing / Test / windows-2.7-none / test_logging_handler_no_client – tests.handlers.logging.logging_tests

@beniwohli beniwohli merged commit d829eae into elastic:master Nov 17, 2020
@beniwohli beniwohli deleted the transaction-ignore-url branch November 17, 2020 14:15
beniwohli added a commit to beniwohli/apm-agent-python that referenced this pull request Sep 14, 2021
* handle transaction_ignore_urls setting [WIP]

* added framework specific tests

(and fixed issues revealed by the tests...)

* implement "outcome" property for transactions and spans

This implements elastic/apm#299.

Additionally, the "status_code" attribute has been added to HTTP spans.

* fix some tests

* change default outcome for spans to "unknown"

* added API functions for setting transaction outcome

* rework outcome API, and make sure it works for unsampled transactions

* fix some tests

* fix a test and add one for testing override behavior

* add an override=False that went forgotten

* expand docs a bit

* implement transaction_ignore_urls [WIP]

* do less work in aiohttp if we're not tracing a transaction

* construct path to json tests in a platform independent way

* fix merge issues

* address review
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.

Implement transaction_ignore_urls
4 participants