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

feat(node/deps): Bump @prisma/instrumentation from 5.19.1 to 5.22.0 #14753

Conversation

aloisklink
Copy link
Contributor

Bump @prisma/instrumentation from 5.19.1 to 5.22.0.

See https://github.com/prisma/prisma/releases/tag/5.22.0 for release notes.

This is to take advantage of the following change in 5.22.0:

In our ongoing effort to stabilize the tracing Preview feature, we’ve made our spans compliant with OpenTelemetry Semantic Conventions for Database Client Calls. This should lead to better compatibility with tools such as DataDog and Sentry [emphasis added by me].

There's also this change in 5.21.0:

The tracing Preview feature now has full support for MongoDB with previously missing functionality now implemented. This is a part of the ongoing effort to stabilize this Preview feature and release it in General Availability.

The @dependabot PR for @prisma/instrumentation updates this to 6.0.0, which unfortunately drops Node.JS v18 support, so it isn't suitable for Sentry v8: #14624


Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).
    • I'm getting a bunch of errors when I run yarn lint && yarn test, but I think they seem to be unrelated to this PR, since this is just a simple dependency update.

Bump [@prisma/instrumentation](https://github.com/prisma/prisma/tree/HEAD/packages/instrumentation) from 5.19.1 to 5.22.0.
- [Release notes](https://github.com/prisma/prisma/releases)
- [Commits](https://github.com/prisma/prisma/commits/5.22.0/packages/instrumentation)

This is to take advantage of:

> In our ongoing effort to stabilize the tracing Preview feature,
> we’ve made our spans compliant with
> OpenTelemetry Semantic Conventions for Database Client Calls.
> This should lead to better compatibility with tools such as
> DataDog and Sentry.

See: https://github.com/prisma/prisma/releases/tag/5.22.0
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

thank you, that makes a lot of sense! We'll release this for v8, on the upcoming v9 release we'll probably bump this further to v6.x!

@mydea mydea changed the title feat(deps): Bump @prisma/instrumentation from 5.19.1 to 5.22.0 feat(node/deps): Bump @prisma/instrumentation from 5.19.1 to 5.22.0 Dec 17, 2024
@mydea mydea merged commit 8077613 into getsentry:develop Dec 17, 2024
145 checks passed
@aloisklink aloisklink deleted the feat/update-prisma-instrumentation-to-5.22.0 branch December 17, 2024 12:59
mydea added a commit that referenced this pull request Dec 17, 2024
This PR adds the external contributor to the CHANGELOG.md file, so that
they are credited for their contribution. See #14753

---------

Co-authored-by: mydea <2411343+mydea@users.noreply.github.com>
Co-authored-by: Francesco Gringl-Novy <francesco.novy@sentry.io>
mydea added a commit that referenced this pull request Dec 17, 2024
#14755)

Backport of #14753:

Bump
[@prisma/instrumentation](https://github.com/prisma/prisma/tree/HEAD/packages/instrumentation)
from 5.19.1 to 5.22.0.

See https://github.com/prisma/prisma/releases/tag/5.22.0 for release
notes.

This is to take advantage of the following change in 5.22.0:

> In our ongoing effort to stabilize the tracing Preview feature, we’ve
made our spans compliant with OpenTelemetry Semantic Conventions for
Database Client Calls. **This should lead to better compatibility with
tools such as DataDog and Sentry** [emphasis added by me].

There's also this change in
[5.21.0](https://github.com/prisma/prisma/releases/tag/5.21.0):

> The `tracing` Preview feature now has full support for MongoDB with
previously missing functionality now implemented. This is a part of the
ongoing effort to stabilize this Preview feature and release it in
General Availability.

The @dependabot PR for `@prisma/instrumentation` updates this to 6.0.0,
which unfortunately drops Node.JS v18 support, so it isn't suitable for
Sentry v8: #14624
@aloisklink
Copy link
Contributor Author

One thing I also just noticed!

It looks like @prisma/instrumentation now sets the db.system attribute automatically: prisma/prisma-engines#5019

This means we might be able to delete the following lines of code which seems to be overriding it:

if (spanJSON.description === 'prisma:engine:db_query') {
span.setAttribute('db.system', 'prisma');
}

However, they set the value to the underlying DB, e.g. if you're using Prisma with a sqlite DB, they'll set db.system = "sqlite", so there is a difference in behavior.

@mydea
Copy link
Member

mydea commented Dec 17, 2024

One thing I also just noticed!

It looks like @prisma/instrumentation now sets the db.system attribute automatically: prisma/prisma-engines#5019

This means we might be able to delete the following lines of code which seems to be overriding it:

if (spanJSON.description === 'prisma:engine:db_query') {
span.setAttribute('db.system', 'prisma');
}

However, they set the value to the underlying DB, e.g. if you're using Prisma with a sqlite DB, they'll set db.system = "sqlite", so there is a difference in behavior.

Ah, thanks for raising this. It's a good question what we expect there 🤔 I'll check in with the team and the folks that work on the performance insights!

@mydea
Copy link
Member

mydea commented Dec 18, 2024

Opened a PR here: #14771

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