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

RUM-4709 feat: decorate network span kind as client #1963

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

ganeshnj
Copy link
Contributor

@ganeshnj ganeshnj commented Jul 19, 2024

What and why?

APM needs to be able to differentiate between a client span vs other span to be able to build features on top of it.

How?

Attach span.kind for all network spans in Trace module.

The ask is for APM spans only, not for RUM spans as they are ingested by RUM BE.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

@ganeshnj ganeshnj requested review from a team as code owners July 19, 2024 09:41
@ganeshnj ganeshnj force-pushed the ganeshnj/feat/client-span branch from 9a1781f to 783407b Compare July 19, 2024 09:44
XCTAssertEqual(
span.tags[SpanTags.errorStack],
"Error Domain=HTTPURLResponse Code=404 \"404 not found\" UserInfo={NSLocalizedDescription=404 not found}"
)
XCTAssertEqual(span.tags.count, 8)
XCTAssertEqual(span.tags.count, 9)
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this update the current tag that was internal before ? Why do we have more tags then before ? We were not sending the kind:internal before on these spans ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting enough we are not sending the kind as of now in open tracing world.

We do this in otel world.

Copy link
Member

Choose a reason for hiding this comment

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

Very interesting ...

@ganeshnj ganeshnj merged commit b58fd25 into develop Jul 22, 2024
16 checks passed
@ncreated ncreated mentioned this pull request Jul 25, 2024
3 tasks
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.

3 participants