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

Add traffic to adservice & recommendataion service #368

Merged

Conversation

dineshg13
Copy link
Member

Fixes #360

Changes

Please provide a brief description of the changes here.

  1. Add .venv to gitignore
  2. Format locustfile.py using black formatter
  3. Add additional routes in loadgenerator to hit frontend ads & recommendation endpoints
  4. Fix container name in docker-compose
  5. Add PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python to ensure recommendation-service comes up.

After above changes , we are able to see traffic to adservice & recommendation service.

Screen Shot 2022-09-15 at 5 46 52 PM

Screen Shot 2022-09-15 at 5 45 46 PM

@dineshg13 dineshg13 requested a review from a team September 15, 2022 21:47
Copy link
Contributor

@puckpuck puckpuck left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for this!

Copy link
Contributor

@mic-max mic-max left a comment

Choose a reason for hiding this comment

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

LGTM, why change the recommendationservice's container_name though? The main reason why these were added was just for readability of the console logs. Trying to keep consistent hyphenated service name for them

@dineshg13
Copy link
Member Author

Because of hyphen in container name, the address of container would change . It needs to be consist with ones defined in .env file .

RECOMMENDATION_SERVICE_ADDR=recommendationservice:${RECOMMENDATION_SERVICE_PORT}

@mic-max
Copy link
Contributor

mic-max commented Sep 16, 2022

Because of hyphen in container name, the address of container would change . It needs to be consist with ones defined in .env file .

RECOMMENDATION_SERVICE_ADDR=recommendationservice:${RECOMMENDATION_SERVICE_PORT}

Were you seeing an error? All the other services addresses work and they use the unhyphenated version, which is the actual root name of the service, not the container name. I believe we want to use the service name here, if we scale up to multiple recommendation service containers running at the same time, I think this would only provide traffic to one or completely break if docker compose decides to name them recommendationservice-1 and recommendationservice-2

@dineshg13
Copy link
Member Author

@mic-max Thanks for catching this. I have revert the container name. Initially i was seeing errors during startup of frontend, but looks like they are just transient ones, and few minutes it seems to be working as expected.

@cartersocha cartersocha merged commit 7484ca8 into open-telemetry:main Sep 16, 2022
@cartersocha
Copy link
Contributor

Thanks @dineshg13 🙏🏼

@austinlparker
Copy link
Member

This PR broke the load generator, primarily through removing locust from requirements.txt.

Reverting PR.

austinlparker added a commit that referenced this pull request Sep 19, 2022
austinlparker added a commit that referenced this pull request Sep 19, 2022
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
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.

[Demo] Adservice & Recommendation Service not getting any traffic
5 participants