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

Add processor for new spans schema #6123

Merged
merged 17 commits into from
Jul 22, 2024
Merged

Add processor for new spans schema #6123

merged 17 commits into from
Jul 22, 2024

Conversation

colin-sentry
Copy link
Member

We are working on a new schema for spans, to better handle arbitrary keys (hashing), improve the sort key, etc.

This listens to the same kafka topic as the existing consumer, but puts data into the new schema.

@colin-sentry colin-sentry requested a review from a team as a code owner July 18, 2024 15:52
@colin-sentry colin-sentry requested review from a team, phacops and evanh July 18, 2024 15:52
rust_snuba/src/processors/mod.rs Outdated Show resolved Hide resolved
rust_snuba/src/processors/spans_v2.rs Outdated Show resolved Hide resolved
Copy link
Member

@evanh evanh left a comment

Choose a reason for hiding this comment

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

This is missing tests but largely looks fine to me.

rust_snuba/src/processors/spans_v2.rs Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jul 18, 2024

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 415 tests with 1 failed, 413 passed and 1 skipped.

View the full list of failed tests

pytest

  • Class name: tests.test_spans_api.TestSpansApi
    Test name: test_count_for_project

    No failure message available

snuba/cli/devserver.py Outdated Show resolved Hide resolved
rust_snuba/src/processors/eap_spans.rs Show resolved Hide resolved
rust_snuba/src/processors/eap_spans.rs Show resolved Hide resolved
rust_snuba/src/processors/eap_spans.rs Show resolved Hide resolved
Copy link
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

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

i assume storage is already merged

@untitaker
Copy link
Member

for the test failures, run cargo install cargo-insta and cargo insta test

@colin-sentry colin-sentry merged commit 794fec5 into master Jul 22, 2024
29 checks passed
@colin-sentry colin-sentry deleted the spans_v2 branch July 22, 2024 19:56
@getsentry-bot
Copy link
Contributor

PR reverted: d60b3e6

getsentry-bot added a commit that referenced this pull request Jul 22, 2024
This reverts commit 794fec5.

Co-authored-by: colin-sentry <161344340+colin-sentry@users.noreply.github.com>
@colin-sentry colin-sentry restored the spans_v2 branch July 22, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants