-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
ref(rust): Remove Python processors in favor of Rust ones #5327
Conversation
…erre/cleanup-python-processors
@@ -100,7 +100,6 @@ struct Function<'a> { | |||
http_method: Option<&'a str>, | |||
is_application: u8, | |||
materialization_version: u8, | |||
module: &'a str, |
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.
how come the rust processor is changing too?
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.
This field is unused and was picked up by the Rust processor when the Python processor wasn't ingesting it.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5327 +/- ##
==========================================
- Coverage 93.10% 90.63% -2.48%
==========================================
Files 839 880 +41
Lines 41357 42759 +1402
Branches 0 288 +288
==========================================
+ Hits 38506 38755 +249
- Misses 2851 3962 +1111
- Partials 0 42 +42 ☔ View full report in Codecov by Sentry. |
Co-Authored-By: phacops <336345+phacops@users.noreply.github.com>
…re/cleanup-python-processors
if let Some(http_method) = from.sentry_tags.http_method { | ||
tag_keys.push("http.method".into()); | ||
tag_values.push(http_method); | ||
} | ||
|
||
if let Some(status_code) = from.sentry_tags.status_code { | ||
tag_keys.push("status_code".into()); | ||
tag_values.push(status_code); | ||
} | ||
|
||
if let Some(transaction_method) = from.sentry_tags.transaction_method { | ||
tag_keys.push("transaction.method".into()); | ||
tag_values.push(transaction_method); | ||
} |
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.
These are not meant to be in tags, just sentry_tags, the Python processor had incorrect behavior.
@@ -270,6 +261,7 @@ impl TryFrom<FromSpanMessage> for Span { | |||
trace_id: from.trace_id, | |||
transaction_id: from.event_id, | |||
transaction_op, | |||
user: from.sentry_tags.user.unwrap_or_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.
Setting a user
column value was somehow forgotten despite the column being there.
@@ -128,7 +128,7 @@ storages: | |||
from_column_table: null | |||
from_column_name: sentry_tags | |||
to_nested_col_table: null | |||
to_nested_col_name: tags | |||
to_nested_col_name: sentry_tags |
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.
We were fetching from tags
instead of sentry_tags
for the sentry_tags
field.
PR reverted: dd479de |
)" This reverts commit 98b9810. Co-authored-by: phacops <336345+phacops@users.noreply.github.com>
No description provided.