Skip to content

Conversation

@beniwohli
Copy link
Contributor

@beniwohli beniwohli commented Jan 10, 2022

What does this pull request do?

@LucaWintergerst pointed out that span compression doesn't work with redis. As it turns out, there were two bugs that caused this:

  1. after we implemented auto-calculation of span destination resource fields, these were no longer available in span compression. Moving the auto-calculation to run before span destination fixed this (e2c9c8f)
  2. redis spans, along with a bunch of other services, were not marked as leaf spans

To ensure that we don't regress on this again, I added a bunch of service-specific span compression tests.

span compression relies on destination resource, so we
have to ensure that auto-calculation runs before span
compression
@ghost
Copy link

ghost commented Jan 10, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-01-11T10:42:04.812+0000

  • Duration: 34 min 4 sec

  • Commit: 981e5cc

Test stats 🧪

Test Results
Failed 0
Passed 10381
Skipped 9603
Total 19984

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /test linters : Run the Python linters only.

  • /test full : Run the full matrix of tests.

  • /test benchmark : Run the APM Agent Python benchmarks tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@basepi basepi left a comment

Choose a reason for hiding this comment

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

Excellent! Redis is definitely a big target of span compression, so I'm glad this was caught fairly quickly!

@beniwohli beniwohli merged commit cb2ca44 into elastic:master Jan 11, 2022
@beniwohli beniwohli deleted the fix-span-compression-redis-et-al branch January 11, 2022 12:23
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.

2 participants