Skip to content

Conversation

@scott20315
Copy link
Contributor

  • pass attrs by value instead of by reference to the otel histogram Record function
    • the histograms in the otel sdk perform an in-place sort on the attrs passed into Record
    • a race condition can occur when separate goroutines concurrently interact with the various redisotel hooks
    • this PR converts the dialhook to a read operation which is consistent with the behavior of the other hooks
  • side benefit of including the error status attr which is another step towards consistency with the other hooks

@monkey92t
Copy link
Collaborator

I don't know about metrics...
It looks like it's just missing handling err.

Copy link
Collaborator

@monkey92t monkey92t left a comment

Choose a reason for hiding this comment

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

seems to be ok.

@monkey92t monkey92t changed the title fix(redisotel): prevent race condition by passing a copy of attrs to Record rather than a reference to the slice fix(redisotel): correct metrics.DialHook attrs Dec 23, 2022
@monkey92t monkey92t merged commit 7c4b924 into redis:master Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants