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

Added path support for HTTP source #2277

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

asifsmohammed
Copy link
Collaborator

@asifsmohammed asifsmohammed commented Feb 14, 2023

Description

  • Adds path option to HTTP source
  • Updated grok e2e to verify the change

Issues Resolved

Resolves #2258

Check List

  • New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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.

Signed-off-by: Asif Sohail Mohammed <[email protected]>
Signed-off-by: Asif Sohail Mohammed <[email protected]>
@asifsmohammed asifsmohammed requested a review from a team as a code owner February 14, 2023 20:22
// TODO: allow customization on URI path for log ingestion
sb.decorator(HTTPSourceConfig.DEFAULT_LOG_INGEST_URI, ThrottlingService.newDecorator(logThrottlingStrategy, logThrottlingRejectHandler));

final String httpSourcePath = sourceConfig.getPath().replace("${PIPELINE_NAME}", pipelineName);
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 use ${pipelineName} to match with other parameter naming conventions.

@@ -22,6 +22,7 @@ source:
## Configurations

* port (Optional) => An `int` between 0 and 65535 represents the port source is running on. Default is ```2021```.
* path (Optional) => A `string` which represents the URI path for log ingestion, and it should start with `/`. Path can contain `${PIPELINE_NAME}` placeholder which will be replaced with pipeline name. Default value is `/log/ingest`.
Copy link
Member

Choose a reason for hiding this comment

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

Please use ${pipelineName} instead of ${PIPELINE_NAME}.

Signed-off-by: Asif Sohail Mohammed <[email protected]>
Signed-off-by: Asif Sohail Mohammed <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2023

Codecov Report

Merging #2277 (fe23191) into main (d53cd96) will decrease coverage by 0.20%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #2277      +/-   ##
============================================
- Coverage     93.81%   93.61%   -0.20%     
- Complexity     1828     1924      +96     
============================================
  Files           225      225              
  Lines          5300     5391      +91     
  Branches        426      427       +1     
============================================
+ Hits           4972     5047      +75     
- Misses          223      240      +17     
+ Partials        105      104       -1     
Impacted Files Coverage Δ
...opensearch/dataprepper/pipeline/ProcessWorker.java 61.53% <0.00%> (-26.93%) ⬇️
...rwarder/discovery/AwsCloudMapPeerListProvider.java 92.85% <0.00%> (-2.39%) ⬇️
.../dataprepper/expression/OperatorConfiguration.java 100.00% <0.00%> (ø)
...taprepper/expression/ParseTreeCoercionService.java 100.00% <0.00%> (ø)
...xpression/LiteralTypeConversionsConfiguration.java 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Collaborator

@kkondaka kkondaka 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.

@asifsmohammed asifsmohammed merged commit e291070 into opensearch-project:main Feb 27, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 27, 2023
* Added path support for HTTP source

Signed-off-by: Asif Sohail Mohammed <[email protected]>
(cherry picked from commit e291070)
asifsmohammed added a commit that referenced this pull request Feb 28, 2023
* Added path support for HTTP source

Signed-off-by: Asif Sohail Mohammed <[email protected]>
(cherry picked from commit e291070)

Co-authored-by: Asif Sohail Mohammed <[email protected]>
mahesh724 pushed a commit to mahesh724/data-prepper that referenced this pull request Mar 3, 2023
* Added path support for HTTP source

Signed-off-by: Asif Sohail Mohammed <[email protected]>
Signed-off-by: mahesh724 <[email protected]>
mahesh724 pushed a commit to mahesh724/data-prepper that referenced this pull request Mar 9, 2023
* Added path support for HTTP source

Signed-off-by: Asif Sohail Mohammed <[email protected]>
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.

Change the path for HTTP source
4 participants