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

docs: Add section for Kiali and distributed tracing OpenShift integration #328

Merged
merged 5 commits into from
Sep 9, 2024

Conversation

josunect
Copy link
Contributor

Add new section in the docs for Kiali and distributed tracing OpenShift integration.

Fixes #324

Copy link

linux-foundation-easycla bot commented Aug 30, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@istio-testing
Copy link
Collaborator

Hi @josunect. Thanks for your PR.

I'm waiting for a istio-ecosystem or istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@josunect josunect force-pushed the issue324_tracing_docs branch from 3507890 to 0314c55 Compare August 30, 2024 12:19
@josunect josunect marked this pull request as ready for review August 30, 2024 12:37
@josunect josunect requested a review from a team as a code owner August 30, 2024 12:37
@nrfox
Copy link
Contributor

nrfox commented Aug 30, 2024

/ok-to-test

@@ -30,6 +30,7 @@
- [Scraping metrics using the OpenShift monitoring stack](#scraping-metrics-using-the-openshift-monitoring-stack)
- [Integrating with Kiali](#integrating-with-kiali)
- [Integrating Kiali with the OpenShift monitoring stack](#integrating-kiali-with-the-openshift-monitoring-stack)
- [Integrating Kiali with OpenShift distributed tracing](#integrating-kiali-with-openshift-distributed-tracing)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you separate configuring Istio and configuring Kiali into two separate sections? Something like

- [Configure tracing with OpenShift distributed tracing](...)
- [Integrating with Kiali](...)
  - [Integrating with the OpenShift monitoring stack](...)
  - [Integrating with OpenShift distributed tracing](...)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you split them up you'll probably need a verification step for Istio which can be 1. generating some traffic and then 2. opening the tracing UI to see traces.

docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated
datasource_uid: "a8d2ef1c-d31c-4de5-a90b-e7bc5252cd00"
```

There is a specific section to setup the Grafana datasource in the Tempo config.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be more instructions for this here?

docs/README.md Outdated

There is a specific section to setup the Grafana datasource in the Tempo config.

Now, we should be able to see traces from Kiali.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a section on what the user should do to see traces in Kiali?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nrfox I have rewritten these sections, see if it makes more sense now

@josunect josunect force-pushed the issue324_tracing_docs branch from b749290 to 08345eb Compare August 30, 2024 14:31
@josunect josunect requested a review from nrfox September 2, 2024 07:10
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Outdated
Comment on lines 653 to 654
kubectl create ns bookinfo
kubectl label ns bookinfo istio-injection=enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

These steps don't actually deploy bookinfo? I think elsewhere we already describe how to deploy bookinfo. Maybe you can just link to that and assume here that you've already deployed it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, right, it is linked now.

@josunect josunect force-pushed the issue324_tracing_docs branch from 749f42c to 9236398 Compare September 9, 2024 11:36
@josunect josunect requested a review from nrfox September 9, 2024 11:44
docs/README.md Outdated
4. Validate the integration: See the traces in the UI

```sh
kubectl get routes -n tempo tempo-sample-query-frontend-tempo.apps-crc.testing
Copy link
Contributor

@nrfox nrfox Sep 9, 2024

Choose a reason for hiding this comment

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

Is this the actual name of the route object? tempo-sample-query-frontend-tempo.apps-crc.testing

Copy link
Contributor

Choose a reason for hiding this comment

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

This part .apps-crc.testing seems like a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! Updated now

@josunect
Copy link
Contributor Author

josunect commented Sep 9, 2024

/retest

josunect and others added 5 commits September 9, 2024 15:44
Signed-off-by: josunect <josune.cordoba@gmail.com>
Co-authored-by: Nick Fox <nick@nrfox.com>

Create a new section

Signed-off-by: josunect <josune.cordoba@gmail.com>
Co-authored-by: Nick Fox <nick@nrfox.com>
Signed-off-by: josunect <josune.cordoba@gmail.com>
Signed-off-by: josunect <josune.cordoba@gmail.com>
Signed-off-by: josunect <josune.cordoba@gmail.com>
@josunect josunect force-pushed the issue324_tracing_docs branch from f1611de to 315c111 Compare September 9, 2024 14:44
@istio-testing istio-testing merged commit e1e581d into istio-ecosystem:main Sep 9, 2024
12 checks passed
openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request Sep 9, 2024
* upstream/main:
  docs: Add section for Kiali and distributed tracing OpenShift integration  (istio-ecosystem#328)
bmangoen pushed a commit to bmangoen/sail-operator that referenced this pull request Sep 10, 2024
…tion (istio-ecosystem#328)

* Add tracing docs

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update docs/README.md

Co-authored-by: Nick Fox <nick@nrfox.com>

Create a new section

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update docs/README.md

Co-authored-by: Nick Fox <nick@nrfox.com>
Signed-off-by: josunect <josune.cordoba@gmail.com>

* Link to install bookinfo

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update route

Signed-off-by: josunect <josune.cordoba@gmail.com>

---------

Signed-off-by: josunect <josune.cordoba@gmail.com>
Co-authored-by: Nick Fox <nick@nrfox.com>
bmangoen pushed a commit to bmangoen/sail-operator that referenced this pull request Sep 10, 2024
…tion (istio-ecosystem#328)

* Add tracing docs

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update docs/README.md

Co-authored-by: Nick Fox <nick@nrfox.com>

Create a new section

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update docs/README.md

Co-authored-by: Nick Fox <nick@nrfox.com>
Signed-off-by: josunect <josune.cordoba@gmail.com>

* Link to install bookinfo

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update route

Signed-off-by: josunect <josune.cordoba@gmail.com>

---------

Signed-off-by: josunect <josune.cordoba@gmail.com>
Co-authored-by: Nick Fox <nick@nrfox.com>
Signed-off-by: bmangoen <bmangoen@redhat.com>
bmangoen pushed a commit to bmangoen/sail-operator that referenced this pull request Oct 17, 2024
…tion (istio-ecosystem#328)

* Add tracing docs

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update docs/README.md

Co-authored-by: Nick Fox <nick@nrfox.com>

Create a new section

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update docs/README.md

Co-authored-by: Nick Fox <nick@nrfox.com>
Signed-off-by: josunect <josune.cordoba@gmail.com>

* Link to install bookinfo

Signed-off-by: josunect <josune.cordoba@gmail.com>

* Update route

Signed-off-by: josunect <josune.cordoba@gmail.com>

---------

Signed-off-by: josunect <josune.cordoba@gmail.com>
Co-authored-by: Nick Fox <nick@nrfox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doc: Add community doc instructions for integrating with openshift tracing
3 participants