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

[SERF-3255] Add Redis client configuration and instrumentation #104

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

Neurostep
Copy link
Contributor

@Neurostep Neurostep commented Apr 19, 2024

Description

In this PR we introduce a new configuration and instrumentation package: Redis.

We use official go-redis client and the dd-trace-go wrapper for instrumentation.

PLEASE NOTE: in this PR we have to upgrade a few widely used libraries, such as dd-trace-go and aws-sdk-go. This is the case because the go-redis wrapper in the dd-trace-go library is supported in the later versions only. Upgrade of the dd-trace-go transitively upgraded the aws-sdk-go. All upstream services will be carefully tested with this change.

Testing considerations

  • Use the Redis client in go-chassis and check traces

Checklist

  • Prefixed the PR title with the JIRA ticket code
  • Performed simple, atomic commits with good commit messages
  • Verified that the commit history is linear and commits are squashed as necessary
  • Thoroughly tested the changes in development and/or staging
  • Updated the README.md as necessary

Related links

@Neurostep Neurostep requested a review from a team as a code owner April 19, 2024 15:13
@Neurostep Neurostep requested a review from fotos April 19, 2024 15:13
Copy link
Contributor

@fotos fotos left a comment

Choose a reason for hiding this comment

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

I did a really quick pass – will remove the documentation next week.

Overall it looks great – left 2 small comments.

Thank you @Neurostep! 🌮

go.mod Show resolved Hide resolved
pkg/logger/redis.go Show resolved Hide resolved
@Neurostep Neurostep force-pushed the maksimt/MLD-6274/redis-client branch from 11a229c to 40f3beb Compare April 22, 2024 10:27
@Neurostep Neurostep requested a review from laynax April 22, 2024 11:51
README.md Show resolved Hide resolved
go.mod Show resolved Hide resolved
pkg/instrumentation/redis/redis.go Outdated Show resolved Hide resolved
pkg/cache/config.go Show resolved Hide resolved
fotos
fotos previously approved these changes Apr 22, 2024
Copy link
Contributor

@fotos fotos left a comment

Choose a reason for hiding this comment

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

👍 LGTM 🚀

pkg/instrumentation/redis/redis.go Outdated Show resolved Hide resolved
pkg/logger/redis.go Show resolved Hide resolved
laynax
laynax previously approved these changes Apr 23, 2024
Copy link
Contributor

@laynax laynax left a comment

Choose a reason for hiding this comment

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

LGTM
We have cache now 🎉

Copy link
Contributor

@laynax laynax left a comment

Choose a reason for hiding this comment

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

LGTM

@Neurostep Neurostep merged commit 6c58521 into main Apr 23, 2024
5 of 6 checks passed
@Neurostep Neurostep deleted the maksimt/MLD-6274/redis-client branch April 23, 2024 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants