-
Notifications
You must be signed in to change notification settings - Fork 223
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
Conversation
(and fixed issues revealed by the tests...)
This implements elastic/apm#299. Additionally, the "status_code" attribute has been added to HTTP spans.
# Conflicts: # elasticapm/instrumentation/packages/tornado.py # tests/contrib/asyncio/tornado/tornado_tests.py # tests/contrib/django/django_tests.py
b1e4651
to
d3a6ba7
Compare
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 great! Excellent testing coverage! 👍
elasticapm/conf/__init__.py
Outdated
@@ -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=[]) | |||
|
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.
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. |
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.
Can you please add path_only
to the docstring?
import pytest | ||
|
||
|
||
def pytest_generate_tests(metafunc): |
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.
Learned something new today! Didn't know about this pytest
feature. 👍
🐛 Flaky test report❕ There are test failures but not known flaky tests. Expand to view the summary
Test stats 🧪
Genuine test errors💔 There are test failures but not known flaky tests, most likely a genuine test failure.
|
* 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
What does this pull request do?
Building on #773, implement
transaction_ignore_urls
Related issues
closes #772