-
Notifications
You must be signed in to change notification settings - Fork 0
fix: support concurrent access in system report endpoint #3
Conversation
TODO: make a Github tag so the new version is available in the Go package registry. |
bb2c844
to
d25cbce
Compare
Previously, concurrent access to the global system report endpoint variable resulted in a data race and triggered the race detector. This commit makes the variable safe for concurrent use. Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
d25cbce
to
34a0b78
Compare
// SetSystemReportEndpoint sets the system report endpoint to the provided | ||
// value or a default | ||
// | ||
// Deprecated: the SetSysetmReportEndpoint shouldn’t be the setter for an | ||
// unexported variable. Use the sysEndpoint.SetOrDefault() method instead. | ||
func SetSystemReportEndpoint(endpoint string) { | ||
systemReportEndpoint.SetOrDefault(endpoint) | ||
} | ||
|
||
// GetSystemReportEndpoint gets the system report endpoint or sets a default | ||
// and returns it | ||
// | ||
// Deprecated: the GetSysetmReportEndpoint shouldn’t be the setter for an | ||
// unexported variable. Use the sysEndpoint.GetOrDefault() method instead. | ||
func GetSystemReportEndpoint() string { | ||
if systemReportEndpoint == "" { | ||
SetSystemReportEndpoint("") | ||
} | ||
return systemReportEndpoint | ||
return systemReportEndpoint.GetOrDefault() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: can we safely remove these functions?
I have only seen these functions being used inside of the armosec/logger-go
package. But seeing how these are exported functions, do we expect clients of armosec/logger-go
to use it?
If not, then I would like to remove the functions and also make sure that the new concurrent system report endpoint is unexported, private to the logger package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do what ever you think, but there is no reason to waist here time
Signed-off-by: Vlad Klokun <vklokun@protonmail.ch>
What this PR changes?
Previously, concurrent access to the global system report endpoint variable resulted in a data race and triggered the race detector.
This commit makes the variable safe for concurrent use.