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

[Profiling] Divide into more packages #107201

Merged

Conversation

danielmitterdorfer
Copy link
Member

With this commit we split the Universal Profiling plugin into three packages:

  • persistence contains everything w.r.t index management
  • rest contains the REST API
  • action contains the transport API

The action / rest structure follows the already established structure in the rest of the code base. We divide this plugin into multiple packages mainly because the different functionalities will be maintained by different teams in the future. This restructuring helps clarify boundaries.

With this commit we split the Universal Profiling plugin into three
packages:

* `persistence` contains everything w.r.t index management
* `rest` contains the REST API
* `action` contains the transport API

The `action` / `rest` structure follows the already established
structure in the rest of the code base. We divide this plugin into
multiple packages mainly because the different functionalities will be
maintained by different teams in the future. This restructuring helps
clarify boundaries.
@danielmitterdorfer danielmitterdorfer added >non-issue :UniversalProfiling/Application Elastic Universal Profiling REST APIs and infrastructure v8.14.0 labels Apr 8, 2024
@elasticsearchmachine elasticsearchmachine added the Team:obs-knowledge Meta label for Observability Knowledge team label Apr 8, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/obs-knowledge-team (Team:obs-knowledge)

assertEquals(response.getTotalSamples(), 3L);

return response;
return new GetStackTracesResponse(
Copy link
Member Author

Choose a reason for hiding this comment

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

I did this because this would have required to make the internal builder class public. As this test focuses on the REST action, it is fine to just construct the response directly (as done in the other test method) and avoid adding a dependency on the builder class.

private final String namePrefix;
private final int version;
private final String generation;
private final OnVersionBump onVersionBump;
private final List<Migration> migrations;

public static ProfilingIndex regular(String name, int version, OnVersionBump onVersionBump) {
static ProfilingIndex regular(String name, int version, OnVersionBump onVersionBump) {
Copy link
Member Author

Choose a reason for hiding this comment

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

While we needed to declare the class itself public, there is no need for code outside the persistence package to create new indices and hence we kept the factory methods package-private.

@danielmitterdorfer
Copy link
Member Author

While the changeset appears large, it's almost entirely file renames and changed package name declarations. I have annotated the changes that I have considered non-obvious.

@danielmitterdorfer danielmitterdorfer merged commit 13f95fd into elastic:main Apr 10, 2024
14 checks passed
@danielmitterdorfer danielmitterdorfer deleted the profiling-package branch April 10, 2024 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue Team:obs-knowledge Meta label for Observability Knowledge team :UniversalProfiling/Application Elastic Universal Profiling REST APIs and infrastructure v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants