-
Notifications
You must be signed in to change notification settings - Fork 527
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
Include distributed docker-compose example #859
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @zalegrala! Added a few comments.
example/docker-compose/local-distributed/tempo-distributed.yaml
Outdated
Show resolved
Hide resolved
example/docker-compose/local-distributed/tempo-distributed.yaml
Outdated
Show resolved
Hide resolved
3c379c2
to
c185805
Compare
c185805
to
0e5d2e5
Compare
Thanks for the review! I believe I addressed the comments, except for the distributed dashboards testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed we are missing the compactor 😳
The compactor is a required component to run Tempo long-term: https://grafana.com/docs/tempo/latest/operations/architecture/
And I querying the local backend doesn't seem to work well. I can successfully query a trace ID for about 10 minutes and then I get "trace not found". So I believe this matches with when the ingesters flush their blocks to the backend.
As long as the block lives on the ingester, the querier can query the ingesters to find the trace. But once the block is flushed and deleted, the querier has to find it in the backend. I guess something is going wrong while flushing...
0e5d2e5
to
fbb3fe2
Compare
fbb3fe2
to
3077f05
Compare
1341bf9
to
681162f
Compare
Thanks for the review. This is looking pretty stable now that minio is in use. |
c56327e
to
ca78b94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Nice work @zalegrala 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🙂 Just some small details left to straighten out. The example works as expected and it's a welcome addition!
ca78b94
to
812c7e2
Compare
812c7e2
to
e967924
Compare
Without this change, there is not an example of how a user might proceed with local testing and development using docker-compose. Here we copy the local docker-compose example and extend it to include the distributed configuration pieces necessary to get a functional example.
e967924
to
8a60c77
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice work. This will be very useful 🙂
Without this change, there is not an example of how a user might proceed with
local testing and development using docker-compose. Here we copy the local
docker-compose example and extend it to include the distributed configuration
pieces necessary to get a functional example.
Fixes #746
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]