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

rework configuration file to deal with telemetry #940

Merged
merged 4 commits into from
Jul 20, 2022
Merged

Conversation

mathnogueira
Copy link
Contributor

This PR reworks how we define telemetry configuration in the config file.

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

@@ -57,6 +57,7 @@ services:
ports:
- "6831:6831/udp"
- "16686:16686"
- "16685:16685"
Copy link
Contributor

@schoren schoren Jul 20, 2022

Choose a reason for hiding this comment

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

nit: you don't need to expose ports for them to work internally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but I usually use the to debug the application as well.

@@ -95,6 +96,8 @@ services:
JAEGER_HOST: jaeger
JAEGER_PORT: 6832
NPM_RUN_COMMAND: api
ports:
- "8081:8081"
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@mathnogueira
Copy link
Contributor Author

I'm merging it. The front-end tests are failing regardless of the changes we are making.

@mathnogueira mathnogueira marked this pull request as ready for review July 20, 2022 14:23
@mathnogueira mathnogueira merged commit 71895e2 into main Jul 20, 2022
@mathnogueira mathnogueira deleted the refactor/config branch July 20, 2022 14:23
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.

2 participants