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

Should set_context with the same context name clobber existing values? #1619

Closed
evanleck opened this issue Nov 18, 2021 · 2 comments · Fixed by #1621
Closed

Should set_context with the same context name clobber existing values? #1619

evanleck opened this issue Nov 18, 2021 · 2 comments · Fixed by #1621
Assignees
Projects
Milestone

Comments

@evanleck
Copy link

It wasn't clear to me from the documentation that calling Scope#set_context with the same name multiple times would clobber existing values, should that be the case? By way of example, here's what I'm doing:

# in my correlation ID middleware:
Sentry.configure_scope do |scope|
  scope.set_context(:request, { correlation_id: env[NAME] })
end

# in my separate session ID middleware:
Sentry.configure_scope do |scope|
  scope.set_context(:request, { session_id: env[Rack::RACK_SESSION][NAME] })
end

Right now, the only context stored in the :request name is the :session_id because that's called later in my middleware stack. Maybe it's just personal opinion, but I would have expected the :request context to contain both :correlation_id and :session_id, but the code calls .merge! and clobbers any existing value using the same name.

Is this by design or should we be able to add to contexts over time? I'd be happy to contribute a change to get that sort of feature added if that would help.

@st0012 st0012 added this to the 4.8.1 milestone Nov 19, 2021
@st0012 st0012 added this to To do in 4.x via automation Nov 19, 2021
@st0012
Copy link
Collaborator

st0012 commented Nov 19, 2021

@evanleck thanks for reporting this. I've added #1621 as a fix.

@evanleck
Copy link
Author

Awesome, thank you @st0012 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4.x
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants