Skip to content

Chunk profiling stacktrace response #96340

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 25, 2023
Merged

Chunk profiling stacktrace response #96340

merged 2 commits into from
May 25, 2023

Conversation

danielmitterdorfer
Copy link
Member

With this commit we implement chunking for the get stacktraces API in the profiling plugin as typical response sizes are dozens of MB large.

@danielmitterdorfer
Chunk profiling stacktrace response
cade5fa 
With this commit we implement chunking for the get stacktraces API in
the profiling plugin as typical response sizes are dozens of MB large.
@danielmitterdorfer danielmitterdorfer added >enhancement v8.9.0 :UniversalProfiling/Application Elastic Universal Profiling REST APIs and infrastructure labels May 25, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/profiling (Team:Universal Profiling)

@elasticsearchmachine
Copy link
Collaborator

Hi @danielmitterdorfer, I've created a changelog YAML for you.

@danielmitterdorfer
Copy link
Member Author

@original-brownbear I've assigned you as a reviewer as you've implemented the underlying infrastructure and I'd appreciate if you could take a look. One remark about the test case: I did not inherit from AbstractChunkedSerializingTestCase because it would require to implement XContent parsing just for tests and our original base class was AbstractWireSerializingTestCase which is sufficient for our use case.

DaveCTurner
DaveCTurner reviewed May 25, 2023
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Looks good to me; I left one question.

original-brownbear
original-brownbear approved these changes May 25, 2023
View reviewed changes
Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM if you're ok with what David points out :)

DaveCTurner
DaveCTurner reviewed May 25, 2023
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM2

@danielmitterdorfer danielmitterdorfer merged commit 905b8fe into elastic:main May 25, 2023
@danielmitterdorfer danielmitterdorfer deleted the profiling-chunked-response branch May 25, 2023 08:28
@danielmitterdorfer
Copy link
Member Author

Thanks both for the quick review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaveCTurner DaveCTurner DaveCTurner approved these changes

@original-brownbear original-brownbear original-brownbear approved these changes

Assignees
No one assigned
Labels
>enhancement :UniversalProfiling/Application Elastic Universal Profiling REST APIs and infrastructure v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danielmitterdorfer @elasticsearchmachine @DaveCTurner @original-brownbear