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

🤖 Enhance Tag Resolution and String Recording in Metrics Query Builder #77988

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

sentry-autofix[bot]
Copy link
Contributor

👋 Hi there! This PR was automatically generated by Autofix 🤖

This fix was triggered by [email protected]

Fixes SENTRY-3A6J

This update introduces several improvements and new functionalities to the metrics query builder and related utilities:

  1. Tag Resolution Handling: In query_builder.py, the resolve_tag_key function now checks if the resolved tag starts with 'unresolved_tag[' or 'invalid_tag['. If so, it returns a NULL column representation using the new create_null_column function.

  2. String Recording Attempts: In utils.py, the resolve function now attempts to record an unknown string up to a maximum of 3 times (MAX_RECORD_ATTEMPTS). If recording fails, it raises a MetricIndexNotFound exception. Metrics are recorded for each attempt and any errors encountered.

  3. Unresolved and Invalid Tag Metrics: The resolve_tag_key function in utils.py now increments specific metrics when encountering unresolved or negative tag keys, returning a formatted string indicating the issue.

  4. Indexer Interface Update: The record method in the Indexer base class (indexer/base.py) now specifies that it should raise an exception if the string cannot be recorded, ensuring consistent error handling across implementations.

If you have any questions or feedback for the Sentry team about this fix, please email [email protected] with the Run ID (see below).

🤓 Stats for the nerds:

Run ID: 925
Prompt tokens: 452319
Completion tokens: 10918
Total tokens: 463237

- Attempt to record unknown strings in the indexer
- Handle unresolved and invalid tag keys gracefully
- Add metrics for monitoring indexer behavior
- Attempt to record unknown strings in the indexer
- Handle unresolved and invalid tag keys gracefully
- Add metrics for monitoring indexer behavior
- Attempt to record unknown strings in the indexer
- Handle unresolved and invalid tag keys gracefully
- Add metrics for monitoring indexer behavior
- Attempt to record unknown strings in the indexer
- Handle unresolved and invalid tag keys gracefully
- Add metrics for monitoring indexer behavior
- Attempt to record unknown strings in the indexer
- Handle unresolved and invalid tag keys gracefully
- Add metrics for monitoring indexer behavior
- Change return type from `int | None` to `int`
- Update docstring to specify exception raising on failure
- Handle unresolved and invalid tags in resolve_tags function
- Create NULL columns for problematic tags
- Ensure queries with unknown tags can still be executed
Copy link

sentry-io bot commented Sep 23, 2024

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: src/sentry/sentry_metrics/utils.py

Function Unhandled Issue
resolve MetricIndexNotFound: Unknown string: 'measurements.frames_slow_rate' sentry.snuba.tasks.delete_sub...
Event Count: 989
resolve MetricIndexNotFound: Unknown string: 'measurements.frames_slow_rate' sentry.snuba.tasks.create_s...
Event Count: 2
resolve MetricIndexNotFound: Unknown string: 'se' sentry....
Event Count: 1
resolve MetricIndexNotFound: Unknown string: 'se' sentry....
Event Count: 1
📄 File: src/sentry/snuba/metrics/query_builder.py (Click to Expand)
Function Unhandled Issue
resolve_tags InvalidParams: The tag key http.method usage has been prohibited by one of the expressions {'match'} ...
Event Count: 2.0k
resolve_tags MetricIndexNotFound: Unknown string: 'measurements.frames_slow_rate' sentry.snuba.tasks.delete_sub...
Event Count: 989
resolve_tags MetricIndexNotFound: Unknown string: 'measurements.frames_slow_rate' sentry.snuba.tasks.create_s...
Event Count: 2
resolve_tags MetricIndexNotFound: Unknown string: 'se' sentry....
Event Count: 1
resolve_tags MetricIndexNotFound: Unknown string: 'se' sentry....
Event Count: 1
---

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 23, 2024
Copy link

codecov bot commented Sep 23, 2024

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
21641 4 21637 206
View the top 3 failed tests by shortest run time
tests.snuba.api.endpoints.test_organization_events_mep.OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithMetricLayer test_non_metrics_tag_with_implicit_format_metrics_dataset
Stack Traces | 5.7s run time
#x1B[1m#x1B[.../api/endpoints/test_organization_events_mep.py#x1B[0m:413: in test_non_metrics_tag_with_implicit_format_metrics_dataset
    assert response.status_code == 400, response.content
#x1B[1m#x1B[31mE   AssertionError: b'{"data":[{"test":"","p50(transaction.duration)":1.0}],"meta":{"fields":{"p50(transaction.duration)":"duration","test...:null},"isMetricsData":true,"isMetricsExtractedData":false,"tips":{},"datasetReason":"unchanged","dataset":"metrics"}}'#x1B[0m
#x1B[1m#x1B[31mE   assert 200 == 400#x1B[0m
#x1B[1m#x1B[31mE    +  where 200 = <Response status_code=200, "application/json">.status_code#x1B[0m
tests.snuba.api.endpoints.test_organization_events_mep.OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithMetricLayer test_non_metrics_tag_with_implicit_format
Stack Traces | 5.75s run time
#x1B[1m#x1B[.../api/endpoints/test_organization_events_mep.py#x1B[0m:395: in test_non_metrics_tag_with_implicit_format
    assert len(response.data["data"]) == 0
#x1B[1m#x1B[31mE   AssertionError: assert 1 == 0#x1B[0m
#x1B[1m#x1B[31mE    +  where 1 = len([{'p50(transaction.duration)': 1.0, 'test': ''}])#x1B[0m
tests.sentry.api.endpoints.test_organization_release_health_data.OrganizationReleaseHealthDataTest test_unknown_filter
Stack Traces | 6.15s run time
#x1B[1m#x1B[.../api/endpoints/test_organization_release_health_data.py#x1B[0m:1463: in test_unknown_filter
    self.get_error_response(
#x1B[1m#x1B[.../sentry/testutils/cases.py#x1B[0m:793: in get_error_response
    assert_status_code(response, status_code)
#x1B[1m#x1B[.../sentry/testutils/asserts.py#x1B[0m:39: in assert_status_code
    assert minimum <= response.status_code < maximum, (
#x1B[1m#x1B[31mE   AssertionError: (200, b'{"start":"2024-09-22T09:00:00Z","end":"2024-09-22T10:00:00Z","intervals":["2024-09-22T09:00:00Z"],"groups":[{"...es":{"sum(sentry.sessions.session)":[0]},"totals":{"sum(sentry.sessions.session)":0.0}}],"meta":[],"query":"foo:123"}')#x1B[0m
#x1B[1m#x1B[31mE   assert 400 <= 200#x1B[0m
#x1B[1m#x1B[31mE    +  where 200 = <Response status_code=200, "application/json">.status_code#x1B[0m

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

@getsantry
Copy link
Contributor

getsantry bot commented Oct 15, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added Stale and removed Stale labels Oct 15, 2024
@getsantry
Copy link
Contributor

getsantry bot commented Nov 6, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added Stale and removed Stale labels Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants