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

Update tempo docs #735

Merged
merged 7 commits into from
Dec 13, 2023
Merged

Update tempo docs #735

merged 7 commits into from
Dec 13, 2023

Conversation

josunect
Copy link
Contributor

@josunect josunect commented Dec 13, 2023

@josunect josunect added the enhancement New feature or request label Dec 13, 2023
@josunect josunect self-assigned this Dec 13, 2023
@josunect josunect marked this pull request as ready for review December 13, 2023 13:09
@josunect josunect requested a review from jmazzitelli December 13, 2023 13:28
Copy link
Contributor

@jmazzitelli jmazzitelli left a comment

Choose a reason for hiding this comment

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

@josunect In addition to my suggestions below, there is one other thing I would like to change but it isn't touched in this PR so github won't let me comment on it directly. It is line 114 that says this:

The `in_cluster_url` Kiali option needs to be set to'
`http://query-frontend.tempo.svc.cluster.local:16685`.

It isn't clear to the reader what this in_cluster_url setting is (and even I'm not 100% sure - I think this is the tracing in_cluster_url, right???). Can we change this to be more specific; something like this (also notice there was an extra ' character here at the end of the line that should be deleted and replaced with the : (colon) character):

The `external_services.tracing.in_cluster_url` Kiali option needs to be set to:
`http://query-frontend.tempo.svc.cluster.local:16685`.

josunect and others added 5 commits December 13, 2023 16:29
Co-authored-by: John Mazzitelli <mazz@redhat.com>
Co-authored-by: John Mazzitelli <mazz@redhat.com>
Co-authored-by: John Mazzitelli <mazz@redhat.com>
Co-authored-by: John Mazzitelli <mazz@redhat.com>
Co-authored-by: John Mazzitelli <mazz@redhat.com>
@josunect
Copy link
Contributor Author

@josunect In addition to my suggestions below, there is one other thing I would like to change but it isn't touched in this PR so github won't let me comment on it directly. It is line 114 that says this:

The `in_cluster_url` Kiali option needs to be set to'
`http://query-frontend.tempo.svc.cluster.local:16685`.

It isn't clear to the reader what this in_cluster_url setting is (and even I'm not 100% sure - I think this is the tracing in_cluster_url, right???). Can we change this to be more specific; something like this (also notice there was an extra ' character here at the end of the line that should be deleted and replaced with the : (colon) character):

The `external_services.tracing.in_cluster_url` Kiali option needs to be set to:
`http://query-frontend.tempo.svc.cluster.local:16685`.

Yes, right, that is the tracing section.

Copy link
Contributor

@jmazzitelli jmazzitelli left a comment

Choose a reason for hiding this comment

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

LGTM

@josunect josunect merged commit 615d723 into kiali:staging Dec 13, 2023
5 checks passed
@josunect josunect deleted the issue6952 branch December 13, 2023 17:03
@josunect josunect mentioned this pull request Jan 15, 2024
7 tasks
hhovsepy pushed a commit to hhovsepy/kiali.io that referenced this pull request Apr 5, 2024
* Update tempo docs

---------

Co-authored-by: John Mazzitelli <mazz@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

document a little more on how to integrate with Tempo
2 participants