Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Support Trace Context HTTP headers #68

Closed
lidel opened this issue Mar 27, 2023 · 8 comments · Fixed by #74
Closed

Support Trace Context HTTP headers #68

lidel opened this issue Mar 27, 2023 · 8 comments · Fixed by #74
Assignees

Comments

@lidel
Copy link
Collaborator

lidel commented Mar 27, 2023

Ref.

Quick thoughts:

  • Whatever we establish here, others in IPFS Ecosystem will see, and assume this is "standard best practice" – we should plan for long term
  • This should be turn-key solution for everyone running boxo/gateway, not just bifrost-gateway
  • bifrost-gateway should ensure tracing info is always present
    • If it is missing in request, bifrost-gateway would create one. (spec)
    • If it is present in request, we update ids in the chain.
      • For Rhea, we will be creating trace parent on LBs (slack thread), but if there is no load balancer in front of bifrost-gateway, then it should initialize trace chain.
### Tasks
- [x] https://github.com/ipfs/bifrost-gateway/pull/74
- [ ]  https://github.com/filecoin-saturn/caboose/pull/87
- [ ] https://github.com/filecoin-saturn/L1-node/issues/337
@lidel
Copy link
Collaborator Author

lidel commented Apr 3, 2023

@hacdias some prior art in ipfs/kubo#9168 + ipfs/kubo#9180, any chance to upstream that to boxo/gateway? Might be less work than we've anticipated 🤞

@hacdias hacdias moved this from 🥞 Todo to 🏃‍♀️ In Progress in IPFS Shipyard Team Apr 4, 2023
@hacdias hacdias moved this from 📋 Backlog to 🏗 In progress in bifrost-gateway Apr 4, 2023
@hacdias hacdias moved this from 🏃‍♀️ In Progress to 🔎 In Review in IPFS Shipyard Team Apr 4, 2023
@lidel
Copy link
Collaborator Author

lidel commented Apr 6, 2023

wip in #74 (review), quick demo by @hacdias

I'm using Kubo @ ipfs/kubo#9801 for testing.

Connected traces of Bifrost Gateway proxying requests to Kubo (PROXY_GATEWAY_URL):

image

Connected traces of Bifrost Gateway proxying IPNS records to Kubo (KUBO_RPC_URL):

image

STRN_ORCHESTRATOR_URL should work since we wrap the caboose transport with otelhttp and caboose already uses the request with context:

https://github.com/filecoin-saturn/caboose/blob/34e6ab0f0e60ed6bef02b06242573aa5ab06bd37/fetcher.go#L215-L219

lidel added a commit to ipfs/boxo that referenced this issue Apr 11, 2023
* feat(examples): wrap handler with OTel propagation
* feat: add tracing sub-package based on Kubo
* docs: update tracing according to comments
* docs(tracing): clarify that kubo is an example

Context:
ipfs-inactive/bifrost-gateway#68
https://www.w3.org/TR/trace-context/

---------

Co-authored-by: Marcin Rataj <[email protected]>
@lidel lidel closed this as completed in #74 Apr 11, 2023
@github-project-automation github-project-automation bot moved this from 🔎 In Review to 🎉 Done in IPFS Shipyard Team Apr 11, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in bifrost-gateway Apr 11, 2023
@lidel
Copy link
Collaborator Author

lidel commented Apr 11, 2023

@willscott @aschmahmann @aarshkshah1992 fyi @hacdias landed support for traceparent header in #74.

When a valid traceparent header is present in the request, traces can be inspected via enabled exporters. Spans may not be perfect right now, but we can add more resolution over time, and once remote http servers also support the Trace Context spec, it will make this a powerful tracing tool for Rhea.

As a Demo, enabling OTEL_TRACES_EXPORTER=jaeger allows finding trace-id from traceparent with Jaeger UI, and one of the views looks like this:

Screenshot 2023-04-12 at 01-02-57 Jaeger UI


Reopening as we need to add tracing to caboose and other types fo HTTP requests sent:

  • confirm that traceparent is applied to all requests sent by bifrost-gateway to other systems
    • KUBO_RPC_URL → routing.go
    • when PROXY_GATEWAY_URL is set → blockstore_proxy.go
    • when STRN_ORCHESTRATOR_URL is set → open PR in caboose that ensures header is forwarded to Saturn L1 with every request?

@lidel lidel reopened this Apr 11, 2023
@lidel lidel moved this from ✅ Done to 🏗 In progress in bifrost-gateway Apr 12, 2023
@lidel
Copy link
Collaborator Author

lidel commented Apr 12, 2023

@hacdias since you've wired this up in both Kubo and bifrost-gateway, mind wiring this up in https://github.com/filecoin-saturn/caboose, ensuring we send traceparent to Saturn when present?

@hacdias
Copy link
Collaborator

hacdias commented Apr 12, 2023

@lidel it should already work. Caboose uses the HTTP Client it is given and we updated it to be wrapped via the otelhttp.NewTransport handler:

https://github.com/ipfs/bifrost-gateway/blob/660ec70a8a7e058c235dfa92e1f50eaab4db1966/blockstore_caboose.go#L63-L89

In addition, Caboose is already creating requests with Context. Assuming the context is being correctly propagated in the Caboose side, the traceparent header should be propagating well. Since Caboose is a library and not a standalone service, we don't need to make further changes, except for perhaps adding useful tracing spans.

The Saturn nodes should, however, be wrapped with the otelhttp.NewHandler such that they parse the incoming requests Trace Context. Things that can be done to future improve the tracing (not sure whose responsibility is it since it falls outside the ipfs org):

Only with all that data we will be able to trace a single requests lifecycle from the browser to the Saturn node(s).

@willscott
Copy link
Collaborator

The Saturn nodes should, however, be wrapped with the otelhttp.NewHandler such that they parse the incoming requests Trace Context.

the saturn nodes are nginx + javascript fwiw, so won't directly be using otelhttp

Filed an issue to track the saturn side: filecoin-saturn/L1-node#337

@BigLep BigLep moved this from 🎉 Done to 🏃‍♀️ In Progress in IPFS Shipyard Team Apr 19, 2023
@lidel
Copy link
Collaborator Author

lidel commented Apr 24, 2023

Confirmed traceparent is forwarded to L1s:

export GODEBUG=http2debug=2
[..]
2023/04/24 16:58:44 http2: Transport encoding header ":method" = "GET"
2023/04/24 16:58:44 http2: Transport encoding header ":path" = "/ipfs/bafybeiaysi4s6lnjev27ln5icwm6tueaw2vdykrtjkwiphwekaywqhcjze?format=car&car-scope=file&depth=1"
2023/04/24 16:58:44 http2: Transport encoding header ":scheme" = "https"
2023/04/24 16:58:44 http2: Transport encoding header "accept" = "application/vnd.ipld.car"
2023/04/24 16:58:44 http2: Transport encoding header "user-agent" = "bifrost-"
2023/04/24 16:58:44 http2: Transport encoding header "traceparent" = "00-65c2a0eb6a8ae8c84a0948ed1ec8718d-004be784a705fd55-01"
2023/04/24 16:58:44 http2: Transport encoding header "accept-encoding" = "gzip"

@lidel lidel closed this as completed May 29, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in bifrost-gateway May 29, 2023
@github-project-automation github-project-automation bot moved this from 🏃‍♀️ In Progress to 🎉 Done in IPFS Shipyard Team May 29, 2023
@lidel
Copy link
Collaborator Author

lidel commented May 29, 2023

Closed, as the remaining work needs to happen in Saturn.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants