newrelic / go-agent

New Relic Go Agent
Apache License 2.0
762 stars 294 forks source link

move fasthttp out of core library, and into integration package #808

Closed iamemilio closed 9 months ago

iamemilio commented 10 months ago

Links

Possible solution to #807

Details

codecov-commenter commented 10 months ago

Codecov Report

Merging #808 (107c958) into develop (a1142ca) will decrease coverage by 0.21%. Report is 2 commits behind head on develop. The diff coverage is 37.06%.

@@             Coverage Diff             @@
##           develop     #808      +/-   ##
===========================================
- Coverage    81.10%   80.89%   -0.21%     
===========================================
  Files          136      139       +3     
  Lines        12393    12545     +152     
===========================================
+ Hits         10051    10148      +97     
- Misses        2062     2111      +49     
- Partials       280      286       +6     
Files Coverage Δ
v3/newrelic/context.go 86.36% <ø> (+26.98%) :arrow_up:
v3/newrelic/segments.go 69.35% <0.00%> (-0.86%) :arrow_down:
v3/integrations/nrfasthttp/instrumentation.go 54.76% <54.76%> (ø)
v3/newrelic/instrumentation.go 59.13% <0.00%> (-9.12%) :arrow_down:
v3/integrations/nrfasthttp/segment.go 41.66% <41.66%> (ø)

... and 6 files with indirect coverage changes

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

iamemilio commented 10 months ago

This needs some revision, we can not copy the request object

iamemilio commented 10 months ago

The more I look at this, the more I realize there may not be a good way to capture a Fasthttp.request. It will always throw an error because of the copy operation, and there is not good way to convert it to an object we can read and use.