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

Adding a new API to get the current transformed pipelines as a JSON #4980

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

sb2k16
Copy link
Contributor

@sb2k16 sb2k16 commented Sep 26, 2024

Description

This PR Is to add a new API to get the body of the transformed pipelines running in the current data prepper instance as an entire JSON string. The API supports both GET and POST methods.

The main purpose is to have a mechanism to get the transformed body of the entire set of pipelines running in the current data prepper instance which should enable faster debugging and troubleshooting purpose.

Check List

  • New functionality includes testing.
  • Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@@ -30,6 +30,7 @@ public class DataPrepperServer {
private static final Logger LOG = LoggerFactory.getLogger(DataPrepperServer.class);
private final HttpServerProvider serverProvider;
private final ListPipelinesHandler listPipelinesHandler;
private final GetTransformedPipelinesBodyHandler getTransformedPipelinesBodyHandler;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be getTransformedPipelinesConfigHander?

@@ -67,6 +70,7 @@ private HttpServer createServer() {

createContext(server, listPipelinesHandler, authenticator, "/list");
createContext(server, shutdownHandler, authenticator, "/shutdown");
createContext(server, getTransformedPipelinesBodyHandler, authenticator, "/getPipelineBody");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above. Should this be /getPipelineTransformedConfig. Who knows we may need /getPipelineConfig in future to get the config before transformation.

Copy link
Member

Choose a reason for hiding this comment

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

How about /pipelines/transformed?

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe this should just be pipelines? I think it makes sense to send the transformed pipelines by default.

Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

Thank you @sb2k16 for this useful feature!

@@ -67,6 +70,7 @@ private HttpServer createServer() {

createContext(server, listPipelinesHandler, authenticator, "/list");
createContext(server, shutdownHandler, authenticator, "/shutdown");
createContext(server, getTransformedPipelinesBodyHandler, authenticator, "/getPipelineBody");
Copy link
Member

Choose a reason for hiding this comment

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

How about /pipelines/transformed?

@@ -67,6 +70,7 @@ private HttpServer createServer() {

createContext(server, listPipelinesHandler, authenticator, "/list");
createContext(server, shutdownHandler, authenticator, "/shutdown");
createContext(server, getTransformedPipelinesBodyHandler, authenticator, "/getPipelineBody");
Copy link
Member

Choose a reason for hiding this comment

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

Or maybe this should just be pipelines? I think it makes sense to send the transformed pipelines by default.

@Override
public void handle(final HttpExchange exchange) throws IOException {
String requestMethod = exchange.getRequestMethod();
if (!requestMethod.equals(HttpMethod.GET) && !requestMethod.equals(HttpMethod.POST)) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's only support GET on this one. It is only getting the pipelines after all.

import java.util.ArrayList;
import java.util.List;

public class GetTransformedPipelinesBodyHandler implements HttpHandler {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if we need Transformed here. The pipeline transformation happens before any pipelines are created. So the transformed pipelines are the real pipelines.

@kkondaka kkondaka merged commit db9a849 into opensearch-project:main Oct 1, 2024
46 of 47 checks passed
sb2k16 added a commit to sb2k16/data-prepper that referenced this pull request Oct 4, 2024
…pensearch-project#4980)

* Adding a new API to get the current transformed pipelines as a JSON

Signed-off-by: Souvik Bose <[email protected]>

* Rename the api and address comments

Signed-off-by: Souvik Bose <[email protected]>

---------

Signed-off-by: Souvik Bose <[email protected]>
Co-authored-by: Souvik Bose <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants