feat(server): Add support for breakdowns ingestion #934

dashed · 2021-02-22T21:40:32Z

Reference: https://getsentry.atlassian.net/browse/VIS-617

Breakdowns are product-defined numbers that are indirectly reported by the client, and are materialized during ingestion. (credit to @jan-auer for this definition)

This PR contains the following changes:

Add breakdowns as a top-level optional field to the event schema.
Normalize events to materialize and emit breakdowns based on a given project breakdown configuration.
For the initial implementation, span ops breakdowns will be emitted for transaction events.A default list will be provided in feat: Return default operation breakdown list from project options sentry#24025

TODO

filter operation names down to http, db, browser, resource (use String:: starts_with())
fetch operation name breakdown list from project options
update tests
update CHANGELOG
settle on breakdown config shape
add breakdowns event field
(optional) Default project breakdown configuration feat: Return default operation breakdown list from project options sentry#24025

relay-general/src/store/normalize/measurements.rs

…tion

dashed · 2021-03-18T17:04:29Z

@untitaker @jan-auer A heads up, I've updated the Measurements type to be generic. In addition, the span op breakdowns will emit timestamp deltas in nanoseconds.

jan-auer

Alright, this looks good. A number of suggestions below, and then going for the final review pass.

relay-general/src/protocol/breakdowns.rs

relay-general/src/store/mod.rs

relay-general/src/protocol/breakdowns.rs

relay-general/src/store/normalize/breakdowns.rs

Co-authored-by: Jan Michael Auer <[email protected]>

k-fish · 2021-03-24T14:38:00Z

tests/integration/test_envelope.py

+    transaction_item = generate_transaction_item()
+    transaction_item.update(
+        {
+            "breakdowns": {"span_ops": {"lcp": {"value": 202.1},}},


Why are you setting this above just to override it here?

@k-fish Unsure what you mean. Events might arrive to Relay with breakdown values; this test is to demonstrate that breakdown objects are recursively merged instead of overwritten.

Ahhh I see. I didn't know the intent was to test the recursive merge. That makes sense then 👍

jan-auer

Thank you, @dashed!

jan-auer · 2021-03-24T16:48:58Z

relay-general/src/protocol/breakdowns.rs

+impl Breakdowns {
+    pub fn is_valid_breakdown_name(name: &str) -> bool {
+        name.chars()
+            .all(|c| matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' | '.'))


Note that this doesn't require the first character to be a letter, which would be a usual requirement.

relay-general/src/protocol/measurements.rs

Co-authored-by: Jan Michael Auer <[email protected]>

* master: fix(server): Remove dependent items from envelope when dropping transaction item (#960) fix(clippy): Fix clippy 1.51.0 warnings (#965) feat(server): Add support for breakdowns ingestion (#934) build: Update schemars and remove workarounds (#961) feat(server): Add rule id to outcomes coming from transaction sampling (#953)

dashed requested a review from a team February 22, 2021 21:40

dashed self-assigned this Feb 22, 2021

k-fish reviewed Feb 22, 2021

View reviewed changes

relay-general/src/store/normalize/measurements.rs Outdated Show resolved Hide resolved

relay-general/src/store/normalize/measurements.rs Outdated Show resolved Hide resolved

relay-general/src/store/normalize/measurements.rs Outdated Show resolved Hide resolved

dashed mentioned this pull request Feb 23, 2021

feat: Return default operation breakdown list from project options getsentry/sentry#24025

Merged

1 task

dashed requested a review from a team February 23, 2021 00:50

dashed force-pushed the generate-operation-breakdowns branch 2 times, most recently from 7af00e6 to 05f7067 Compare February 24, 2021 18:49

dashed added 15 commits February 26, 2021 14:38

impl Deref and DerefMut for Measurements

039b125

add explicit type

39e72c4

pub Measurement in protocol

ac9e3c8

co-locate measurements normalization in its own module

929ea7a

emit operations breakdown measurements

cf3d9fc

filter operation breakdowns to: http, db, browser, and resource

3b4f781

use ops.time.{} naming scheme

c3619d7

discard other breakdowns

c0b17d9

merge intervals last

2ca2bf0

meta: Changelog

57cb8a3

fix clippy errors

4b77574

emit measurements based on project operation_name_breakdown configura…

dbdcb0b

…tion

add description

f829ad1

add basic measurements normalization tests

d977cbe

test_emit_ops_breakdown_measurements

ad5f422

dashed force-pushed the generate-operation-breakdowns branch from 05f7067 to ad5f422 Compare February 26, 2021 21:18

dashed added 5 commits February 26, 2021 16:37

emit operation breakdown measurement last

83b3066

make clippy happy

0e22513

should use prefixed operation name

75cbcdc

add integration test

95b7bc9

make format

42840f4

dashed marked this pull request as ready for review February 26, 2021 22:54

stupid floats

1c28250

update breakdown tests

07d0512

update test

48f0e15

dashed mentioned this pull request Mar 19, 2021

chore: Remove try-catch wrapper when processing measurements getsentry/snuba#1768

Closed

jan-auer changed the title ~~feat(server): Add support for breakdowns ingestion.~~ feat(server): Add support for breakdowns ingestion Mar 22, 2021

add trait EmitBreakdowns

7138283

jan-auer reviewed Mar 22, 2021

View reviewed changes

dashed and others added 8 commits March 22, 2021 17:38

revert breakdowns to use measurements f64

b1b0f20

Measurements' storage shall only be f64

8ec64b0

prefer bevity lifetime

706ee14

Update relay-general/src/protocol/breakdowns.rs

e496f28

Co-authored-by: Jan Michael Auer <[email protected]>

address feedback

c2b450b

consolidate breakdown config types and impl with breakdown normalization

2efa3bf

add doc strings

368bc16

clarify docs

77d3127

dashed requested a review from jan-auer March 22, 2021 22:37

dashed mentioned this pull request Mar 22, 2021

feat(breakdowns): Schema change for span op breakdowns getsentry/snuba#1761

Merged

k-fish reviewed Mar 24, 2021

View reviewed changes

jan-auer approved these changes Mar 24, 2021

View reviewed changes

dashed and others added 7 commits March 24, 2021 15:09

Update relay-general/src/protocol/measurements.rs

25f15e3

Co-authored-by: Jan Michael Auer <[email protected]>

use into_inner

5604d8f

Merge branch 'master' into generate-operation-breakdowns

b994a7e

update CHANGELOG.md

7646f59

ensure breakdown names are not empty and starts with a letter

567fcae

fix comments

4e8ccc4

Merge branch 'master' into generate-operation-breakdowns

a5884d0

dashed mentioned this pull request Mar 25, 2021

feat(breakdowns): Add span_op_breakdowns fields to transactions and discover datasets. getsentry/snuba#1763

Merged

1 task

dashed merged commit 810d953 into master Mar 25, 2021

dashed deleted the generate-operation-breakdowns branch March 25, 2021 13:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(server): Add support for breakdowns ingestion #934

feat(server): Add support for breakdowns ingestion #934

dashed commented Feb 22, 2021 •

edited

Loading

dashed commented Mar 18, 2021

jan-auer left a comment

k-fish Mar 24, 2021

dashed Mar 24, 2021

k-fish Mar 24, 2021

jan-auer left a comment

jan-auer Mar 24, 2021

feat(server): Add support for breakdowns ingestion #934

feat(server): Add support for breakdowns ingestion #934

Conversation

dashed commented Feb 22, 2021 • edited Loading

TODO

dashed commented Mar 18, 2021

jan-auer left a comment

Choose a reason for hiding this comment

k-fish Mar 24, 2021

Choose a reason for hiding this comment

dashed Mar 24, 2021

Choose a reason for hiding this comment

k-fish Mar 24, 2021

Choose a reason for hiding this comment

jan-auer left a comment

Choose a reason for hiding this comment

jan-auer Mar 24, 2021

Choose a reason for hiding this comment

dashed commented Feb 22, 2021 •

edited

Loading