Add github.com/gofiber/fiber trace instrumentation #880

WLun001 · 2021-07-12T07:11:34Z

close #843

Co-authored-by: ReneWerner87 <[email protected]>

instrumentation/github.com/gofiber/fiber/otelfiber/fiber_test.go

instrumentation/github.com/gofiber/fiber/README.md

instrumentation/github.com/gofiber/fiber/otelfiber/config.go

pellared · 2021-07-12T15:02:40Z

Could you please also add a line to CHANGELOG.md?

instrumentation/github.com/gofiber/fiber/otelfiber/fiber.go

instrumentation/github.com/gofiber/fiber/otelfiber/example/docker-compose.yml

instrumentation/github.com/gofiber/fiber/otelfiber/config.go

Co-authored-by: Anthony Mirabella <[email protected]>

instrumentation/github.com/gofiber/fiber/otelfiber/fiber_test.go

pellared

Was instrumentation/github.com/gofiber/fiber/README.md not supposed to be removed?

Reference https://github.com/open-telemetry/opentelemetry-go-contrib/pull/880/files#r667995107

WLun001 · 2021-07-13T10:18:01Z

@pellared sorry I mistakenly removed the wrong README

pellared · 2021-07-13T10:43:20Z

instrumentation/github.com/gofiber/fiber/otelfiber/example/server.go

+	oteltrace "go.opentelemetry.io/otel/trace"
+)
+
+var tracer = otel.Tracer("fiber-server")


@open-telemetry/go-approvers Would it be better to recommend:

adding sdktrace.WithResource(resource.NewWithAttributes(semconv.SchemaURL, semconv.ServiceNameKey.String("fiber-server"))), to sdktrace.NewTracerProvider call

creating the new span inside getUser using oteltrace.SpanFromContext()
?

Reference: https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/instrumentation/net/http/otelhttp/example/server/server.go

A service name should definitely be provided in the resource given to the tracer provider initialization.

instrumentation/github.com/gofiber/fiber/otelfiber/fiber.go

CHANGELOG.md

Co-authored-by: Robert Pająk <[email protected]>

pellared · 2021-07-13T11:51:41Z

instrumentation/github.com/gofiber/fiber/otelfiber/fiber.go

+)
+
+// Middleware returns fiber handler which will trace incoming requests.
+func Middleware(service string, opts ...Option) fiber.Handler {


@open-telemetry/go-approvers side question:
do we have an example of documentation that tells what attributes are being set for given instrumentation?

CHANGELOG.md

instrumentation/github.com/gofiber/fiber/otelfiber/fiber.go

Co-authored-by: Robert Pająk <[email protected]>

pellared · 2021-07-13T15:32:50Z

@WLun001 You need to add the new modules to .github/dependabot.yml

instrumentation/github.com/gofiber/fiber/otelfiber/example/go.mod

instrumentation/github.com/gofiber/fiber/otelfiber/go.mod

Co-authored-by: Robert Pająk <[email protected]>

pellared · 2021-07-14T10:06:20Z

@WLun001 Looks like you need to go mod tidy and go fmt.

WLun001 · 2021-07-14T11:01:11Z

just ran go mod tidy, go fmt and make precommit locally, should be ok now

codecov · 2021-07-14T11:43:43Z

Codecov Report

Merging #880 (831878b) into main (76dbf66) will increase coverage by 9.7%.
The diff coverage is 96.5%.

❗ Current head 831878b differs from pull request most recent head 92aa894. Consider uploading reports for the commit 92aa894 to get more accurate results

@@           Coverage Diff           @@
##            main    #880     +/-   ##
=======================================
+ Coverage   69.6%   79.4%   +9.7%     
=======================================
  Files         77      64     -13     
  Lines       4955    2789   -2166     
=======================================
- Hits        3451    2215   -1236     
+ Misses      1365     441    -924     
+ Partials     139     133      -6

Impacted Files	Coverage Δ
...tation/github.com/gofiber/fiber/otelfiber/fiber.go	`96.0% <96.0%> (ø)`
...ation/github.com/gofiber/fiber/otelfiber/config.go	`100.0% <100.0%> (ø)`
instrumentation/net/http/otelhttp/transport.go	`78.5% <0.0%> (-13.8%)`	⬇️
exporters/metric/dogstatsd/dogstatsd.go	`75.8% <0.0%> (-5.0%)`	⬇️
instrumentation/net/http/otelhttp/handler.go	`72.6% <0.0%> (-4.3%)`	⬇️
exporters/metric/cortex/cortex.go	`69.8% <0.0%> (-3.4%)`	⬇️
propagators/b3/context.go	`66.6% <0.0%> (-3.4%)`	⬇️
propagators/jaeger/context.go	`66.6% <0.0%> (-3.4%)`	⬇️
detectors/aws/eks/detector.go	`32.7% <0.0%> (-3.0%)`	⬇️
exporters/metric/cortex/utils/config_utils.go	`89.4% <0.0%> (-2.2%)`	⬇️
... and 72 more

Aneurysm9 · 2021-07-23T16:38:35Z

instrumentation/github.com/gofiber/fiber/otelfiber/example/server.go

+	oteltrace "go.opentelemetry.io/otel/trace"
+)
+
+var tracer = otel.Tracer("fiber-server")


A service name should definitely be provided in the resource given to the tracer provider initialization.

instrumentation/github.com/gofiber/fiber/otelfiber/fiber.go

WLun001 · 2021-08-23T06:48:28Z

any updates on this PR?

# Conflicts: # .github/dependabot.yml # CHANGELOG.md

efectn · 2021-10-04T17:13:09Z

Do you have any plan to add offical Fiber support?

pellared · 2021-10-05T06:01:27Z

I do not think it will be merged. See: #1100

However, I hope that all the code reviews were helpful.

MrAlias · 2021-10-12T18:39:48Z

Thanks for the contribution. According to our new instrumentation policy, we are no longer accepting new instrumentation to the project as we do not have the developer bandwidth to support it. If you are able, please consider maintaining this instrumentation in your own repository and listing it in the the OpenTelemetry Registry.

WLun001 · 2021-10-13T02:18:32Z

@pellared thanks for the code review, very helpful

WLun001 and others added 2 commits July 12, 2021 15:09

add gofiber support

fa1eb2a

Co-authored-by: ReneWerner87 <[email protected]>

update example

34c7b50

WLun001 requested review from Aneurysm9, dashpole, evantorrie, jmacd, MadVikingGod, MrAlias, paivagustavo, pellared and XSAM as code owners July 12, 2021 07:11

WLun001 mentioned this pull request Jul 12, 2021

🚀 add Fiber to OpenTelemetry repo gofiber/fiber#1416

Closed

pellared reviewed Jul 12, 2021

View reviewed changes

instrumentation/github.com/gofiber/fiber/otelfiber/fiber_test.go Outdated Show resolved Hide resolved

instrumentation/github.com/gofiber/fiber/README.md Outdated Show resolved Hide resolved

instrumentation/github.com/gofiber/fiber/otelfiber/config.go Outdated Show resolved Hide resolved

Aneurysm9 reviewed Jul 12, 2021

View reviewed changes

WLun001 and others added 2 commits July 13, 2021 11:09

Apply suggestions from code review

ad5cb65

Co-authored-by: Anthony Mirabella <[email protected]>

apply suggestions

a935b9a

WLun001 requested review from Aneurysm9 and pellared July 13, 2021 03:39

pellared reviewed Jul 13, 2021

View reviewed changes

instrumentation/github.com/gofiber/fiber/otelfiber/fiber_test.go Outdated Show resolved Hide resolved

apply suggestions for test file

e07589d

WLun001 requested a review from pellared July 13, 2021 09:26

pellared changed the title ~~add Fiber~~ Add github.com/gofiber/fiber trace instrumentation Jul 13, 2021

pellared reviewed Jul 13, 2021

View reviewed changes

remove readme

908d912

pellared reviewed Jul 13, 2021

View reviewed changes

WLun001 and others added 2 commits July 13, 2021 19:20

Update CHANGELOG.md

dd8d2ce

Co-authored-by: Robert Pająk <[email protected]>

remove unreachable if block

c420aef

pellared reviewed Jul 13, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

instrumentation/github.com/gofiber/fiber/otelfiber/fiber.go Show resolved Hide resolved

pellared reviewed Jul 13, 2021

View reviewed changes

instrumentation/github.com/gofiber/fiber/otelfiber/fiber.go Outdated Show resolved Hide resolved

WLun001 and others added 2 commits July 13, 2021 20:11

Update CHANGELOG.md

058b488

Co-authored-by: Robert Pająk <[email protected]>

update comment and headers construction

0febf4d

pellared approved these changes Jul 13, 2021

View reviewed changes

add new module to dependabot

797e9fd

pellared reviewed Jul 13, 2021

View reviewed changes

instrumentation/github.com/gofiber/fiber/otelfiber/example/go.mod Outdated Show resolved Hide resolved

pellared reviewed Jul 13, 2021

View reviewed changes

instrumentation/github.com/gofiber/fiber/otelfiber/go.mod Outdated Show resolved Hide resolved

Apply suggestions from code review

c9d7935

Co-authored-by: Robert Pająk <[email protected]>

go mod tidy and go fmt

831878b

Aneurysm9 reviewed Jul 23, 2021

View reviewed changes

WLun001 added 2 commits September 14, 2021 22:23

Merge branch 'main' into fiber

f7a72e4

# Conflicts: # .github/dependabot.yml # CHANGELOG.md

add recordError, update sdk to RC3

92aa894

MrAlias closed this Oct 12, 2021

WLun001 deleted the fiber branch October 28, 2021 14:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add github.com/gofiber/fiber trace instrumentation #880

Add github.com/gofiber/fiber trace instrumentation #880

WLun001 commented Jul 12, 2021

pellared commented Jul 12, 2021

pellared left a comment •

edited

Loading

WLun001 commented Jul 13, 2021 •

edited

Loading

pellared Jul 13, 2021

Aneurysm9 Jul 23, 2021

pellared Jul 13, 2021 •

edited

Loading

pellared commented Jul 13, 2021 •

edited

Loading

pellared commented Jul 14, 2021

WLun001 commented Jul 14, 2021

codecov bot commented Jul 14, 2021 •

edited

Loading

Aneurysm9 Jul 23, 2021

WLun001 commented Aug 23, 2021 •

edited

Loading

efectn commented Oct 4, 2021

pellared commented Oct 5, 2021

MrAlias commented Oct 12, 2021

WLun001 commented Oct 13, 2021

Add github.com/gofiber/fiber trace instrumentation #880

Add github.com/gofiber/fiber trace instrumentation #880

Conversation

WLun001 commented Jul 12, 2021

pellared commented Jul 12, 2021

pellared left a comment • edited Loading

Choose a reason for hiding this comment

WLun001 commented Jul 13, 2021 • edited Loading

pellared Jul 13, 2021

Choose a reason for hiding this comment

Aneurysm9 Jul 23, 2021

Choose a reason for hiding this comment

pellared Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

pellared commented Jul 13, 2021 • edited Loading

pellared commented Jul 14, 2021

WLun001 commented Jul 14, 2021

codecov bot commented Jul 14, 2021 • edited Loading

Codecov Report

Aneurysm9 Jul 23, 2021

Choose a reason for hiding this comment

WLun001 commented Aug 23, 2021 • edited Loading

efectn commented Oct 4, 2021

pellared commented Oct 5, 2021

MrAlias commented Oct 12, 2021

WLun001 commented Oct 13, 2021

pellared left a comment •

edited

Loading

WLun001 commented Jul 13, 2021 •

edited

Loading

pellared Jul 13, 2021 •

edited

Loading

pellared commented Jul 13, 2021 •

edited

Loading

codecov bot commented Jul 14, 2021 •

edited

Loading

WLun001 commented Aug 23, 2021 •

edited

Loading