Skip to content

feat(instrumentation): add ability to filter span by PrismaLayerType #20113

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

Merged
merged 33 commits into from
Mar 10, 2025

Conversation

overbit
Copy link
Contributor

@overbit overbit commented Jul 7, 2023

Add ability to filter out some of the instrumentation span generated by prisma

Related #14640

#14640 (comment)

@overbit overbit requested a review from a team as a code owner July 7, 2023 14:54
@overbit overbit requested review from SevInf and aqrln and removed request for a team July 7, 2023 14:54
@CLAassistant
Copy link

CLAassistant commented Jul 7, 2023

CLA assistant check
All committers have signed the CLA.

@janpio
Copy link
Contributor

janpio commented Jul 7, 2023

(The failing test in CI is unrelated)

@overbit
Copy link
Contributor Author

overbit commented Jul 24, 2023

@janpio all tests are passing now after sync this branch with main

@aqrln
Copy link
Member

aqrln commented Jul 24, 2023

The code changes look good, thanks so much for the contribution! This definitely needs to be tested, would you be able to add some tests, or do you need help with this?

@overbit
Copy link
Contributor Author

overbit commented Jul 24, 2023

The code changes look good, thanks so much for the contribution! This definitely needs to be tested, would you be able to add some tests, or do you need help with this?

@aqrln I could add some unit tests but I'm not sure what is the strategy here. The only other tests are pretty basic.

Any suggestions? Otherwise I'm happy if you guys want to add specific tests.

@overbit
Copy link
Contributor Author

overbit commented Jul 26, 2023

@aqrln tests added. There are failing tests in the ci but they shouldn't be related to these changes

@janpio
Copy link
Contributor

janpio commented Jul 27, 2023

(Retriggered a run for the failing tests)

Copy link
Member

@aqrln aqrln left a comment

Choose a reason for hiding this comment

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

This looks very solid, thank you for adding the tests! What I was having in mind was rather adding some integration tests in packages/client/test/functional/tracing (sorry for not making this clear, it's not easy to navigate our test structure) but these unit tests should also be very useful. We can add the integration tests ourselves because I don't want to burden you with more work and I'd like to merge this PR soon — but if you are interested and want to, feel free to look into adding them, just let me know!

@overbit overbit requested a review from aqrln July 31, 2023 14:31
@overbit
Copy link
Contributor Author

overbit commented Aug 3, 2023

@janpio @aqrln any update on this? We really need it otherwise we will disable the prisma instrumentation all together due to the amount of segments created

@aqrln
Copy link
Member

aqrln commented Aug 3, 2023

@overbit sorry for not bringing this to the finish line in time for 5.1.0 but this will land in 5.2.0 — I'll make sure to review and merge it soon.

@nomego
Copy link

nomego commented Aug 19, 2024

Any progress?

@overbit
Copy link
Contributor Author

overbit commented Dec 17, 2024

@SevInf apologies but I haven't got any time to proceed and resolve your comments. Feel free to take over from here

@lewinskimaciej
Copy link

lewinskimaciej commented Dec 19, 2024

For people who got here like me after a while, as it seems Prisma team won't implement any filtering on their own, you can achieve what you want by creating a custom SpanProcessor(/BatchSpanProcessor) like this:

import {
  ReadableSpan,
  SpanExporter,
  BatchSpanProcessor,
} from '@opentelemetry/sdk-trace-base';

export class FilteringSpanProcessor extends BatchSpanProcessor {
  private spansToFilterOut: RegExp;

  constructor(exporter: SpanExporter, spanPatternToFilterOut: string) {
    super(exporter);
    this.spansToFilterOut = new RegExp(spanPatternToFilterOut);
  }

  onEnd(span: ReadableSpan): void {
    // Filter out spans based on custom criteria
    if (this.shouldExport(span)) {
      super.onEnd(span);
    }
  }

  private shouldExport(span: ReadableSpan): boolean {
    return !this.spansToFilterOut.test(span.name);
  }
}

and registering it with NodeSdk:

new FilteringSpanProcessor(
        traceExporter,
        // this will filter out anything that starts with prisma: but is not prisma:client:operation or prisma:engine:query/db_query spans
        '^(prisma:(?!client:operation|engine:(query|db_query)).*)', 
      ),

or filter it out directly in your OTel agent that ingests spans but that often is not possible if you use some paid services.

@FGoessler FGoessler assigned FGoessler and unassigned SevInf Mar 4, 2025
@FGoessler FGoessler added this to the 6.5.0 milestone Mar 4, 2025
@FGoessler
Copy link
Contributor

Thanks again @overbit for this nice PR! I took the liberty to apply the suggested changes, improve the test suite and slightly updated the functionality to now also support regex based filtering. ;)

Filters can be supplied on the PrismaInstrumentation constructor. E.g.:

new PrismaInstrumentation({ 
  ignoreSpanTypes: [
    /prisma:client:ser.*/, 
    'prisma:client:connect', 
    'prisma:engine:query',
  ], 
})

Note that if you filter out a parent span all its child spans will also be filtered out. E.g. prisma:engine:query has various child spans types as well as prisma:client:transaction and prisma:client:operation.

Tracing docs can be found here: https://www.prisma.io/docs/orm/prisma-client/observability-and-logging/opentelemetry-tracing

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 10, 2025
@FGoessler FGoessler dismissed SevInf’s stale review March 10, 2025 10:29

Changes addressed

@FGoessler FGoessler merged commit d44d9ea into prisma:main Mar 10, 2025
240 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer topic: tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.