Skip to content

Conversation

basepi
Copy link
Contributor

@basepi basepi commented Mar 24, 2022

What does this pull request do?

Adds the new span_stack_trace_min_duration, which is replacing span_frames_min_duration with a little tweak to the behavior of -1 and 0 as described in the spec.

Related issues

Closes #1369

basepi added 4 commits March 22, 2022 11:26
All the old tests were updated to use span_stack_trace_min_duration, and I added a test which tests the override of span_frames_min_duration
@basepi basepi requested a review from beniwohli March 24, 2022 17:34
@basepi
Copy link
Contributor Author

basepi commented Mar 24, 2022

@beniwohli This interacts with your timedelta stuff in #1488. The conflicts should be easy enough, but this work will need to be updated to use timedelta by whoever merges second.

@basepi
Copy link
Contributor Author

basepi commented Mar 24, 2022

Added @bmorelli25 as a reviewer to look over my docs changes

@ghost
Copy link

ghost commented Mar 24, 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-03-29T17:20:07.272+0000

  • Duration: 20 min 53 sec

Test stats 🧪

Test Results
Failed 0
Passed 4773
Skipped 3209
Total 7982

💚 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!)

@basepi
Copy link
Contributor Author

basepi commented Mar 28, 2022

/test

Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

A few doc comments for your consideration.

Copy link
Contributor

@beniwohli beniwohli left a comment

Choose a reason for hiding this comment

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

LGTM, nice work!

@basepi basepi merged commit 44eca82 into elastic:main Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants