Skip to content

fix(rpc): Define measurements columns for RPC#80866

Merged
Zylphrex merged 5 commits into
masterfrom
txiao/fix/define-measurements-columns-for-rpc
Nov 18, 2024
Merged

fix(rpc): Define measurements columns for RPC#80866
Zylphrex merged 5 commits into
masterfrom
txiao/fix/define-measurements-columns-for-rpc

Conversation

@Zylphrex

Copy link
Copy Markdown
Member

We need to ensure that in the RPC layer, we still correctly handle the sentry defined measurements.

We need to ensure that in the RPC layer, we still correctly handle the sentry
defined measurements.
@Zylphrex Zylphrex requested a review from a team November 15, 2024 22:20
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 15, 2024
@codecov

codecov Bot commented Nov 15, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #80866       +/-   ##
===========================================
+ Coverage   57.51%   78.43%   +20.91%     
===========================================
  Files        7200     7211       +11     
  Lines      319181   319817      +636     
  Branches    43946    44026       +80     
===========================================
+ Hits       183580   250845    +67265     
+ Misses     130845    62581    -68264     
- Partials     4756     6391     +1635     

@Zylphrex Zylphrex left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@wmak FYI, these tests will fail under RPC mode for the time being

Comment on lines +416 to +418
if resolved.search_type != "string" or not isinstance(
resolved.proto_definition, AttributeKey
):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

heads up this change; https://github.com/getsentry/sentry/pull/80829/files#diff-ca53615e4a5d771b967f49cd366caf62d188212ece593a146cf4b31de578015aR67-R79
will mean we don't need to do the isinstance part of the check anymore

simple_measurements_field("messaging.message.body.size"),
simple_measurements_field("messaging.message.receive.latency"),
simple_measurements_field("messaging.message.retry.count"),
simple_measurements_field("http.response_content_length"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh god

@Zylphrex Zylphrex enabled auto-merge (squash) November 18, 2024 17:59
@Zylphrex Zylphrex merged commit 33237ba into master Nov 18, 2024
@Zylphrex Zylphrex deleted the txiao/fix/define-measurements-columns-for-rpc branch November 18, 2024 18:18
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants