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.

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

Added a docker-compose for hotrod e2e test #5740

Merged
merged 25 commits into from
Jul 17, 2024
Merged

Added a docker-compose for hotrod e2e test #5740

merged 25 commits into from
Jul 17, 2024

Conversation

hellspawn679
Copy link
Contributor

Which problem is this PR solving?

Description of the changes

  • added docker-compose file to test hotrod so that is can connect to jaeger

How was this change tested?

Checklist

@hellspawn679 hellspawn679 requested a review from a team as a code owner July 13, 2024 07:18
@hellspawn679 hellspawn679 requested a review from joe-elliott July 13, 2024 07:18
@dosubot dosubot bot added the docker Pull requests that update Docker code label Jul 13, 2024
@codecov Codecov
Copy link

codecov bot commented Jul 13, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.66%. Comparing base (c68136e) to head (9d82e79).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #5740   +/-   ##
=======================================
  Coverage   96.66%   96.66%           
=======================================
  Files         341      341           
  Lines       16451    16451           
=======================================
  Hits        15902    15902           
  Misses        361      361           
  Partials      188      188
Flag Coverage Δ
badger_v1 8.05% <ø> (ø)
badger_v2 1.90% <ø> (ø)
cassandra-3.x-v1 16.61% <ø> (ø)
cassandra-3.x-v2 1.82% <ø> (ø)
cassandra-4.x-v1 16.61% <ø> (ø)
cassandra-4.x-v2 1.82% <ø> (ø)
elasticsearch-6.x-v1 18.77% <ø> (-0.02%) ⬇️
elasticsearch-7.x-v1 18.83% <ø> (ø)
elasticsearch-8.x-v1 19.03% <ø> (ø)
elasticsearch-8.x-v2 1.90% <ø> (+0.01%) ⬆️
grpc_v1 9.52% <ø> (ø)
grpc_v2 7.47% <ø> (ø)
kafka 9.74% <ø> (ø)
memory_v2 1.90% <ø> (ø)
opensearch-1.x-v1 18.89% <ø> (ø)
opensearch-2.x-v1 18.88% <ø> (-0.02%) ⬇️
opensearch-2.x-v2 1.90% <ø> (+0.01%) ⬆️
unittests 95.08% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yurishkuro
Copy link
Member

I suggest you explain your plan, doesn't seem it's going in the right direction. We already have all the pieces you included in this PR

@hellspawn679
Copy link
Contributor Author

@yurishkuro my plan is to build a local hotrod image(localhost:5000/jaegertracing/example-hotrod:${GITHUB_SHA:-latest}) and test it using docker-compose(that way we won't have any error for port 4318) then we will hit the frontend end point and get the hotrod logs and get the tracesID from there

@hellspawn679 hellspawn679 requested a review from yurishkuro July 13, 2024 20:17
fixed
0ef8541 
Signed-off-by: mehul gautam <[email protected]>
@hellspawn679 hellspawn679 requested a review from yurishkuro July 14, 2024 00:23
yurishkuro
yurishkuro reviewed Jul 14, 2024
View reviewed changes
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

you have UI submodule affected in this PR, please remove.

examples/hotrod/docker-compose.yml Outdated Show resolved Hide resolved
hellspawn679 and others added 5 commits July 14, 2024 20:40
@hellspawn679
Merge branch 'main' into hotrod
45c96a3
fixed
e159787 
Signed-off-by: mehul gautam <[email protected]>
fixed
85cd331 
Signed-off-by: mehul gautam <[email protected]>
fixed
a00f808 
Signed-off-by: mehul gautam <[email protected]>
fixed
f2f4b95 
Signed-off-by: mehul gautam <[email protected]>
@hellspawn679 hellspawn679 requested a review from yurishkuro July 14, 2024 16:02
@yurishkuro yurishkuro added the changelog:test Change that's adding missing tests or correcting existing tests label Jul 14, 2024
hellspawn679 and others added 2 commits July 15, 2024 03:01
@hellspawn679 hellspawn679 requested a review from yurishkuro July 14, 2024 22:36
yurishkuro
yurishkuro reviewed Jul 17, 2024
View reviewed changes
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

lgtm, a couple open questions

mehul gautam and others added 2 commits July 17, 2024 04:39
return 0 if jq fails
bd5a86e 
Signed-off-by: mehul gautam <[email protected]>
@hellspawn679 hellspawn679 requested a review from yurishkuro July 17, 2024 05:51
fixed
50b014f 
Signed-off-by: mehul gautam <[email protected]>
@hellspawn679 hellspawn679 requested a review from yurishkuro July 17, 2024 06:25
@yurishkuro yurishkuro merged commit edfee78 into jaegertracing:main Jul 17, 2024
43 checks passed
@yurishkuro
Copy link
Member

Thanks!

yurishkuro
yurishkuro reviewed Jul 18, 2024
View reviewed changes
scripts/hotrod-integration-test.sh

bash scripts/build-upload-a-docker-image.sh -c example-hotrod -d examples/hotrod -p "${platforms}"
Copy link
Member

Choose a reason for hiding this comment

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

this line should not have been deleted. It needs to run as the last step of the script.

FlamingSaint pushed a commit to FlamingSaint/jaeger that referenced this pull request Jul 20, 2024
@hellspawn679 @FlamingSaint
Added a docker-compose for hotrod e2e test (jaegertracing#5740)
e01e670 
## Which problem is this PR solving?
- jaegertracing#5735

## Description of the changes
- added docker-compose file to test hotrod so that is can connect to
jaeger

## How was this change tested?
- 

## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: mehul gautam <[email protected]>
Co-authored-by: mehul gautam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurishkuro yurishkuro yurishkuro approved these changes

@joe-elliott joe-elliott Awaiting requested review from joe-elliott joe-elliott is a code owner automatically assigned from jaegertracing/jaeger-maintainers

Assignees
No one assigned
Labels
changelog:test Change that's adding missing tests or correcting existing tests docker Pull requests that update Docker code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hellspawn679 @yurishkuro