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

Prisma instrumentation #1517

Merged
merged 7 commits into from
Mar 1, 2023
Merged

Conversation

bizob2828
Copy link
Member

@bizob2828 bizob2828 commented Feb 27, 2023

Proposed Release Notes

  • Added support for Prisma.
    • Captures spans for queries. It names them based on the model and action.(i.e. Datastore/statement/Prisma/user/create)
    • Captures database metrics for queries.
    • Provides connection between application and database server via service maps.

Huge shoutout to @osmanmrtacar for the original contribution 🙏🏻

Links

Details

You can see here the span names. It also connects the app with db.

screenshot 2023-02-27 at 5 01 23 PM

When using prisma.$executeRaw we will name the spans as Datastore/statement/Prisma/$executeRaw(<sql cmd>)/<database name>

screenshot 2023-02-27 at 5 04 07 PM

I also had to extend the datastore shim which I plan on writing unit tests for to take advantage of the inContext method to set the params for the database host/port/db.

Lastly, the original PR instrumented _request but _executeRequest seems more appropriate as it's a higher level function that gets called

@codecov
Copy link

codecov bot commented Feb 28, 2023

Codecov Report

Merging #1517 (f19ca0f) into main (2424fd2) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1517      +/-   ##
==========================================
+ Coverage   96.39%   96.41%   +0.01%     
==========================================
  Files         196      197       +1     
  Lines       38198    38350     +152     
  Branches       23       23              
==========================================
+ Hits        36822    36974     +152     
  Misses       1376     1376              
Flag Coverage Δ
esm-unit-tests-14.x 47.57% <ø> (ø)
esm-unit-tests-16.x 92.14% <ø> (ø)
esm-unit-tests-18.x 92.14% <ø> (ø)
integration-tests-14.x 77.58% <94.73%> (+<0.01%) ⬆️
integration-tests-16.x 77.68% <94.73%> (+<0.01%) ⬆️
integration-tests-18.x 77.68% <94.73%> (+<0.01%) ⬆️
unit-tests-14.x 90.29% <100.00%> (+0.05%) ⬆️
unit-tests-16.x 90.35% <100.00%> (+0.05%) ⬆️
unit-tests-18.x 90.33% <100.00%> (+0.05%) ⬆️
versioned-tests-14.x 75.20% <100.00%> (+0.01%) ⬆️
versioned-tests-16.x 76.62% <100.00%> (+<0.01%) ⬆️
versioned-tests-18.x 76.62% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/instrumentation/@prisma/client.js 100.00% <100.00%> (ø)
lib/instrumentations.js 100.00% <100.00%> (ø)
lib/metrics/names.js 100.00% <100.00%> (ø)
lib/shim/datastore-shim.js 99.40% <100.00%> (+<0.01%) ⬆️
lib/symbols.js 100.00% <100.00%> (ø)
lib/metrics/normalizer.js 97.63% <0.00%> (+0.05%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jmartin4563 jmartin4563 self-assigned this Mar 1, 2023
@jmartin4563 jmartin4563 removed their assignment Mar 1, 2023
database_name: parsedUrl.path && decodeURIComponent(parsedUrl.path.split('/')[0])
}
} catch (err) {
logger.warn('Failed to parse connection string for %s: %s', provider, err.message)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to prefer format strings over template literals?

Copy link
Member Author

Choose a reason for hiding this comment

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

not really. template strings are newer and sometimes I'm just used to doing this when logging

Copy link
Contributor

@jordigh jordigh left a comment

Choose a reason for hiding this comment

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

Seems alright, should probably just keep the one added symbol with all of the others we already have.

@@ -5,7 +5,7 @@

'use strict'

/* eslint sonarjs/cognitive-complexity: ["error", 52] -- TODO: https://issues.newrelic.com/browse/NEWRELIC-5252 */
/* eslint sonarjs/cognitive-complexity: ["error", 57] -- TODO: https://issues.newrelic.com/browse/NEWRELIC-5252 */
Copy link
Member Author

Choose a reason for hiding this comment

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

revert

@bizob2828 bizob2828 merged commit 6bf87f5 into newrelic:main Mar 1, 2023
@github-actions github-actions bot mentioned this pull request Mar 8, 2023
@bizob2828 bizob2828 deleted the prisma-instrumentation branch August 28, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants