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

[tracing] Propagate TraceID to the backend. Create a new setupEnv span. Fix span hierarchy #604

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

sgirones
Copy link
Contributor

@sgirones sgirones commented Aug 31, 2022

This PR tackles three main issues:

  • Propagate TraceID to the backend: this change enables Datadog APM to link the SDK traces with the backend traces and logs
  • Use a dedicated span for the setup actions: this change traces the setup actions performed by every test suite and links the setup span to the suite span

TODO:

  • Fix span hierarchy: Actions performed inside each test case are linked to the suite span instead of the test span. The test span created in the beforeEach hook needs to be activated before the test code runs.

@changeset-bot
Copy link

changeset-bot bot commented Aug 31, 2022

⚠️ No Changeset found

Latest commit: 78d5497

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR
Some errors occurred when validating the changesets config:
The package or glob expression "@xata.io/client" specified in the `linked` option does not match any package in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "@xata.io/codegen" specified in the `linked` option does not match any package in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2022

size-limit report 📦

Path Size
packages/client/dist/index.mjs 10.92 KB (+0.39% 🔺)
packages/client/dist/index.cjs 11.98 KB (+0.57% 🔺)
packages/codegen/dist/index.mjs 2.01 MB (0%)
packages/codegen/dist/index.cjs 2.01 MB (0%)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2022

Your pull request has been published to npm.

You can install it by running:

npm install @xata.io/[email protected]

To test the CLI, run:

npx @xata.io/[email protected]

@sgirones sgirones marked this pull request as ready for review September 1, 2022 08:10
@sgirones sgirones requested a review from a team as a code owner September 1, 2022 08:10
@@ -1,19 +1,32 @@
export type AttributeDictionary = Record<string, string | number | boolean | undefined>;

export type SetAttributesFn = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this one is unused it seems

Copy link
Member

Choose a reason for hiding this comment

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

Yeah these are publicly exported types, so doesn't matter if we don't use them internally.

Copy link
Member

@SferaDev SferaDev left a comment

Choose a reason for hiding this comment

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

@sgirones Give it a try please!

@sgirones
Copy link
Contributor Author

sgirones commented Sep 6, 2022

Looking good! The hierarchy of spans is maintained till the backend 👌

One downside with the final refactor is that we lost the parent Span, and now we have a bunch of top-level spans that belong to the same suite but we don't have a way to point to "all of them".

@SferaDev
Copy link
Member

SferaDev commented Sep 6, 2022

I will look at it! Thanks @sgirones

@SferaDev SferaDev marked this pull request as draft October 28, 2022 10:23
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