Skip to content
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

feat(gw): tracing spans per response type #8841

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

lidel
Copy link
Member

@lidel lidel commented Apr 1, 2022

This PR extends #8595 with spans for each response type from #8758
Adds more visibility into how long generic lookup takes vs producing specific response type.

Demo

CAR Block File Directory
2022-04-01_01-46 block_2022-04-01_01-47 2022-04-01_01-49 dir_2022-04-01_01-48

Adds more visibility into how long generic lookup takes vs producing
specific response type.
@lidel lidel requested a review from guseggert April 1, 2022 00:40
@lidel
Copy link
Member Author

lidel commented Apr 1, 2022

@guseggert to make this easier to review, this PR points at your branch.

If this looks good, feel free to merge, and we can continue in #8595

@lidel lidel added this to the go-ipfs 0.13 milestone Apr 1, 2022
@@ -70,7 +70,7 @@ test_go_fmt:
TEST_GO += test_go_fmt

test_go_lint: test/bin/golangci-lint
golangci-lint run ./...
golangci-lint run --timeout=3m ./...
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helps with flaky CI.

func (i *gatewayHandler) serveRawBlock(w http.ResponseWriter, r *http.Request, blockCid cid.Cid, contentPath ipath.Path, begin time.Time) {
blockReader, err := i.api.Block().Get(r.Context(), contentPath)
func (i *gatewayHandler) serveRawBlock(w http.ResponseWriter, r *http.Request, resolvedPath ipath.Resolved, contentPath ipath.Path, begin time.Time) {
ctx, span := tracing.Span(r.Context(), "Gateway", "ServeRawBlock", trace.WithAttributes(attribute.String("path", resolvedPath.String())))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this attribute, since the path is already on the parent span?

Copy link
Member Author

@lidel lidel Apr 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is yes: one may want to inspect this span on its own + this path is "resolved one", so it adds additional context when /ipns/ was used for initial request (no need to look at all Resolve operations before it to find out final path)

@lidel lidel merged commit 60b3809 into feat/tracing Apr 4, 2022
@lidel lidel deleted the feat/tracing-gateway-response-types branch April 4, 2022 15:13
lidel added a commit that referenced this pull request Apr 4, 2022
* add deprecation warning when tracer plugins are loaded
* add response format attribute to span in gateway handler
* add note about tracing's experimental status in godoc
* add nil check for TTL when adding name span attrs
* add basic sharness test for integration with otel collector
* add nil check in UnixFSAPI.processLink
* test: sharness check all json objs for swarm span
* add env var docs to docs/environment-variables.md
* chore: pin the otel collector version
* add tracing spans per response type (#8841)
* docs: tracing with jaeger-ui

Co-authored-by: Marcin Rataj <lidel@lidel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants