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

chore: add observability section #338

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

vmihailenco
Copy link
Contributor

This was briefly discussed with @mortensi , but let me know what changes you would like to see.

Copy link
Contributor

@andy-stark-redis andy-stark-redis left a comment

Choose a reason for hiding this comment

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

The text looks fine but I don't know enough to review the technical info.

@dwdougherty dwdougherty requested a review from mortensi June 24, 2024 15:50
@dwdougherty
Copy link
Collaborator

@mortensi Will you please take a look at this PR and comment as to its technical correctness?

@mortensi
Copy link
Member

mortensi commented Jun 28, 2024

The sample is ok, and does not produce errors. However, the user needs a direction to effectively set up the integration.

Is it possible to elaborate more?

@mortensi
Copy link
Member

Maybe in this phase it's possible to point the user to the full example here

https://github.com/redis/go-redis/blob/master/example/otel/README.md

Once the docs are restructured, the full example could be taken to a separate, dedicated "Observability" page.

@vmihailenco vmihailenco force-pushed the chore/go-observability branch from 98ac9e5 to 7bbde9e Compare July 18, 2024 11:56
@vmihailenco
Copy link
Contributor Author

@mortensi I've followed your suggestion and added a link to the GitHub example. Please take another look.

@andy-stark-redis andy-stark-redis merged commit 2cd739d into redis:main Jul 25, 2024
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.

4 participants