Skip to content

feat(tracing): add telemetry span kind#5526

Open
gacevicljubisa wants to merge 6 commits into
masterfrom
feat/telemetry-span-kind
Open

feat(tracing): add telemetry span kind#5526
gacevicljubisa wants to merge 6 commits into
masterfrom
feat/telemetry-span-kind

Conversation

@gacevicljubisa

Copy link
Copy Markdown
Member

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

P2P spans are now tagged with client/server span kinds so trace UIs render request/response relationships correctly, and all span attributes are namespaced under swarm.* (e.g. address → swarm.chunk.address, tag_id → swarm.tag.id, success → swarm.operation.success) to align with OpenTelemetry semantic-convention naming. The go.mod indirect dependencies are also consolidated into a single block with no version changes. These are trace-annotation changes only, no behavioral impact.

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

AI Disclosure

  • This PR contains code that has been generated by an LLM.
  • I have reviewed the AI generated code thoroughly.
  • I possess the technical expertise to responsibly review the code generated in this PR.

Comment thread pkg/pushsync/pushsync.go
defer action.Cleanup()

spanInner.SetAttributes(attribute.String("peer_address", peer.String()))
spanInner.SetAttributes(attribute.String("swarm.peer.address", peer.String()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The server handler tags the remote as swarm.peer.sender_address while here the client tags the remote peer as the generic swarm.peer.address. Should we have here something like swarm.peer.target_address ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch on asymmetry. And after some digging, my opinion is that it would be better to drop the role, and use added span kind we just introduced. That way on the client push span, the peer is the destination, and on the server handler span, the peer is the source. This way, spans are role agnostic. What do you think?

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