Implement unmarshal metrics with jsoniter

[atingchen]

Description: I implement unmarshal metrics with jsoniter iterator parser, compare with jsonpb unmarshaller, jsoniter does not use reflect, more efficient, less GC pressure.

Link to tracking Issue: [#4986 ] Testing: unit test. construct a trace message, fill all the fields, use jsonpb marshal to json bytes, use jsoniter unmarshal to struct, test assert there are equal.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: atingchen / name: look (fb1b1de8bda8817d0022dbe2e0f4c06d909408a9, 0fbed74f2d74a16c58a3d1e53cac4603d87db07b, 6b32d706db126aa638d79fedc076bfee61f80de0, d0c9a50fea8bc71045caa938cdc1af44d8896f16)

[github-actions[bot]]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[hanjm]

still valid

[codecov[bot]]

Codecov Report

Merging #5433 (417c99d) into main (9e90e25) will increase coverage by 0.28%. The diff coverage is 98.47%.

@@            Coverage Diff             @@
##             main    #5433      +/-   ##
==========================================
+ Coverage   91.44%   91.72%   +0.28%     
==========================================
  Files         196      197       +1     
  Lines       11958    12475     +517     
==========================================
+ Hits        10935    11443     +508     
- Misses        808      816       +8     
- Partials      215      216       +1     

Impacted Files	Coverage Δ
pdata/pmetric/json.go	97.94% <98.11%> (-2.06%)	:arrow_down:
pdata/internal/json/common.go	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[tigrannajaryan]

@atingchen thank you for the PR. I started reviewing it but I see that the build is failing. Can you please fix the build to make sure all github actions pass so that we can then review the PR. Faster performance is great, however we need to make there is very thorough coverage by the tests since this deals with data received externally.

[atingchen]

@tigrannajaryan Thank you for your comments. I will fix it as soon as possible

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: atingchen / name: look (2a3b60f8487ff2291d756493ba59833b0dd0ea63, 0bb9c1c47bc1179542bc72f1ebef443d5c05799b, f67bd70f3a89abdb9f599395896c1ae31ae98819, 5045e6d6ae4338c2a5c4b725cc5583f2ddae5c99, ff7fc033b54fd4f42fb68a17dce475ce3363a65d, 6d8da86eaeeebfcb9de6c024039d2793b02a0cc6)

[tigrannajaryan]

Please make sure EasyCLA passes and there are no irrelevant changes.

[atingchen]

It is caused by the error of executing the command merge, and it has been solved now

[tigrannajaryan]

Please make sure the build passes. Preferably you should do this before creating a PR. make with various targets can be run locally.

[tigrannajaryan]

@open-telemetry/collector-approvers please review. Question: do we need any additional precautions to ensure we did not break OTLP/JSON compatibility or the tests that are introduced are good enough?

[Aneurysm9]

@open-telemetry/collector-approvers please review. Question: do we need any additional precautions to ensure we did not break OTLP/JSON compatibility or the tests that are introduced are good enough?

Is it possible to maintain the current implementation somewhere and add tests that demonstrate that the same JSON documents are unmarshalled to equivalent pdata structures by both implementations? I think that would give me the most confidence in the new implementation. It would also allow us to introduce this change with a feature gate, which would allow users who ran into issues we didn't test for to quickly revert to the old implementation.

[atingchen]

Is it possible to maintain the current implementation somewhere and add tests that demonstrate that the same JSON documents are unmarshalled to equivalent pdata structures by both implementations? I think that would give me the most confidence in the new implementation. It would also allow us to introduce this change with a feature gate, which would allow users who ran into issues we didn't test for to quickly revert to the old implementation.

Thanks for your suggestion, I'll think about how to implement it

[atingchen]

@Aneurysm9 I found the function 'NewJSONMarshaler' is unused in our project. And I provide new function 'NewJSONITERUnmarshaler' which will return a model.Unmarshaler which use jsoniter. I'm happy to use new features feature gate to modify the receiver later

[bogdandrutu]

I found the function 'NewJSONMarshaler' is unused in our project. And I provide new function 'NewJSONITERUnmarshaler' which will return a model.Unmarshaler which use jsoniter. I'm happy to use new features feature gate to modify the receiver later

Please don't add a new API for this, and do as we did for trace which is to reuse the other API NewJSONMarshaler. We cannot easily use a feature gate here, because of that we should just replace that implementation and document this.

[atingchen]

I found the function 'NewJSONMarshaler' is unused in our project. And I provide new function 'NewJSONITERUnmarshaler' which will return a model.Unmarshaler which use jsoniter. I'm happy to use new features feature gate to modify the receiver later

Please don't add a new API for this, and do as we did for trace which is to reuse the other API NewJSONMarshaler. We cannot easily use a feature gate here, because of that we should just replace that implementation and document this.

Thank you for your suggestion. I have Modified it.