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

feat: open telemetry tracing #87

Merged
merged 2 commits into from
Apr 26, 2023
Merged

feat: open telemetry tracing #87

merged 2 commits into from
Apr 26, 2023

Conversation

hacdias
Copy link
Contributor

@hacdias hacdias commented Apr 12, 2023

Adds Open Telemetry tracing spans. Context: ipfs/bifrost-gateway#68 (comment).

This is not necessarily required. On the screenshot below you see the tracing result from the Bifrost Gateway up until the HTTP request Caboose makes to the Saturn node. What this PR adds is the Caboose.* trace spans. The HTTP GET already worked.

Screenshot 2023-04-12 at 11 19 18

@lidel @willscott not sure where is the most value to put the spans here, but this seems like a good start. Feel free to add/remove some.

Copy link
Contributor

@lidel lidel left a comment

Choose a reason for hiding this comment

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

We should remove Blockstore. prefix, at least for Fetch, which is an Exchange interface, and not Blockstore one.

@hacdias
Copy link
Contributor Author

hacdias commented Apr 25, 2023

I just removed the prefix Blockstore. We already have Caboose in there anyways, and that's also the name of the object. I think Pool.FetchResource should remain with Pool prefix.

@hacdias hacdias requested a review from lidel April 25, 2023 09:20
Copy link
Contributor

@lidel lidel left a comment

Choose a reason for hiding this comment

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

lgtm, should be good enough for now
@willscott / @aarshkshah1992 ok to merge? we can add more spans in the future, if needed

@willscott willscott merged commit 5e676ee into main Apr 26, 2023
12 of 16 checks passed
@willscott willscott deleted the feat/tracing-spans branch April 26, 2023 07:03
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 this pull request may close these issues.

None yet

3 participants