-
Notifications
You must be signed in to change notification settings - Fork 395
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
[target-allocator] use json iterator to marshal in server #1410
Conversation
624d0ff
to
bc47716
Compare
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.
after this change goes out, should we upstream a change to the collector that uses a json unmarshal?
cmd/otel-allocator/server/server.go
Outdated
w.Header().Set("Content-Type", "application/json") | ||
err := s.jsonMarshaller.NewEncoder(w).Encode(data) | ||
if err != nil { |
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 our tests already cover this case? i.e. is this change just going to work with what we do in the collector? using specifically the yaml unmarshal function?
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.
That yaml unmarshal function is for a response from the scrape config handler, which doesn't use the jsonHandler
function. Here is the marshaling logic for the scrape config endpoint.
The jsonHandler()
function is used for the job and targets handlers, both of which have test cases which ensure that this marshaling is still compatible with json.Unmarshal()
:
err = json.Unmarshal(bodyBytes, &itemResponse) err = json.Unmarshal(bodyBytes, &jobs)
bc47716
to
c6a1236
Compare
Latest benchmarks |
fixes #1336
This time, server functionality is covered by unit tests. Below are the results of the benchmarks, showing using the standard json library (std) vs jsoniter.
test_results.txt