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

Bring all span data under pii control in rust_tracing integration #3778

Closed
wants to merge 1 commit into from

Conversation

antonpirker
Copy link
Member

Before only data that was set in on_record was protected by our PII controls. Now also the span data that is add in on_new_span is prtected by PII controls.


Thank you for contributing to sentry-python! Please add tests to validate your changes, and lint your code using tox -e linters.

Running the test suite on your PR might require maintainer approval. The AWS Lambda tests additionally require a maintainer to add a special label, and they will fail until this label is added.

Copy link

codecov bot commented Nov 14, 2024

❌ 132 Tests Failed:

Tests completed Failed Passed Skipped
13842 132 13710 4117
View the top 3 failed tests by shortest run time
tests.integrations.quart.test_quart test_cli_commands_raise
Stack Traces | 0.001s run time
.../integrations/quart/test_quart.py:304: in test_cli_commands_raise
    app = quart_app_factory()
.../integrations/quart/test_quart.py:32: in quart_app_factory
    app = Quart(__name__)
.tox/py3.11-quart-v0.19/lib/python3.11.../site-packages/quart/app.py:338: in __init__
    self.add_url_rule(
.tox/py3.11-quart-v0.19/lib/python3.11.../flask/sansio/scaffold.py:47: in wrapper_func
    return f(self, *args, **kwargs)
.tox/py3.11-quart-v0.19/lib/python3.11.../flask/sansio/app.py:641: in add_url_rule
    if "OPTIONS" not in methods and self.config["PROVIDE_AUTOMATIC_OPTIONS"]:
E   KeyError: 'PROVIDE_AUTOMATIC_OPTIONS'
tests.integrations.quart.test_quart test_cli_commands_raise
Stack Traces | 0.001s run time
.../integrations/quart/test_quart.py:304: in test_cli_commands_raise
    app = quart_app_factory()
.../integrations/quart/test_quart.py:32: in quart_app_factory
    app = Quart(__name__)
.tox/py3.12-quart-v0.19/lib/python3.12.../site-packages/quart/app.py:338: in __init__
    self.add_url_rule(
.tox/py3.12-quart-v0.19/lib/python3.12.../flask/sansio/scaffold.py:47: in wrapper_func
    return f(self, *args, **kwargs)
.tox/py3.12-quart-v0.19/lib/python3.12.../flask/sansio/app.py:641: in add_url_rule
    if "OPTIONS" not in methods and self.config["PROVIDE_AUTOMATIC_OPTIONS"]:
E   KeyError: 'PROVIDE_AUTOMATIC_OPTIONS'
tests.integrations.quart.test_quart test_cli_commands_raise
Stack Traces | 0.001s run time
.../integrations/quart/test_quart.py:304: in test_cli_commands_raise
    app = quart_app_factory()
.../integrations/quart/test_quart.py:32: in quart_app_factory
    app = Quart(__name__)
.tox/py3.12-quart-latest/lib/python3.12.../site-packages/quart/app.py:338: in __init__
    self.add_url_rule(
.tox/py3.12-quart-latest/lib/python3.12.../flask/sansio/scaffold.py:47: in wrapper_func
    return f(self, *args, **kwargs)
.tox/py3.12-quart-latest/lib/python3.12.../flask/sansio/app.py:641: in add_url_rule
    if "OPTIONS" not in methods and self.config["PROVIDE_AUTOMATIC_OPTIONS"]:
E   KeyError: 'PROVIDE_AUTOMATIC_OPTIONS'

To view more test analytics, go to the Test Analytics Dashboard
Got feedback? Let us know on Github

Copy link
Contributor

@matt-codecov matt-codecov left a comment

Choose a reason for hiding this comment

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

could you rename send_sensitive_data to include_tracing_fields? i worry users will not know what we could possibly mean by "sensitive data" because this is mostly data they explicitly want tracing to record. include_tracing_fields is a more accurate name

@matt-codecov
Copy link
Contributor

#3780 i implemented my name-change suggestion here. also moved the should_send_default_pii() if include_tracing_fields is None else include_tracing_fields check to a helper function with an explanatory comment. otherwise no changes

@antonpirker
Copy link
Member Author

Closing in favor of #3780

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants