kfaraz commented on code in PR #17170:
URL: https://github.com/apache/druid/pull/17170#discussion_r2209227633
##########
processing/src/main/java/org/apache/druid/java/util/emitter/service/ServiceMetricEvent.java:
##########
@@ -197,7 +198,7 @@ public ServiceMetricEvent build(ImmutableMap<String,
String> serviceDimensions)
return new ServiceMetricEvent(
createdTime,
serviceDimensions,
- userDims,
+ new HashMap<>(userDims),
Review Comment:
I think we should have gone here with making an immutable copy of the map in
the builder after all.
Otherwise, there is a potential race condition.
Since the event builder can be reused, it is possible that the user dims of
an event are updated even before the event has been emitted (since emission is
always async). This would be even more likely to happen when there are multiple
emitters.
The other alternative would be to disallow reuse of the event builder, but
it does offer some convenience when emitting multiple related events.
`ServiceMetricEvent` should be immutable anyway.
cc: @gianm , @kgyrtkirk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]