Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

feat: add requireParentSpan to typeorm instrumentation #270

Closed
wants to merge 11 commits into from

Conversation

ragrag
Copy link

@ragrag ragrag commented Mar 31, 2024

adds requireParentSpan to typeorm instrumentation

@ragrag ragrag requested a review from a team as a code owner March 31, 2024 10:59
import { TypeormInstrumentationConfig } from '../types';

export function shouldSkipInstrumentation(config: TypeormInstrumentationConfig) {
return config.requireParentSpan === true && trace.getSpan(context.active()) === undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

return config.requireParentSpan && trace.getSpan(context.active()) === undefined; should work, there is no need for the === true check

Copy link
Contributor

Choose a reason for hiding this comment

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

Add tests to cover this new behavior.
see

for example

@ragrag
Copy link
Author

ragrag commented Apr 1, 2024

@mottibec thanks,

build & test are failing locally though also on ci e.g: https://github.com/aspecto-io/opentelemetry-ext-js/actions/runs/8495994068/job/23272882049

i suspect that's due to lockfiles being in gitignore and some bumped ^ packages have mismatching types.
could you provide them in upstream?

@mottibec
Copy link
Contributor

mottibec commented Apr 2, 2024

It looks like the daily tests haven't been running for a while, i'm not sure what the issue is there. Anyway, I approved your test runs, but it fails on some module node supported version. This is probably not related to your change. To be honest, no one is actively maintaining this repository. I'm not sure how to help with that, but if you are able to fix it, it would be amazing. Otherwise, it would probably be left hanging there.

@ragrag ragrag requested a review from mottibec April 3, 2024 00:23
@ragrag
Copy link
Author

ragrag commented Apr 3, 2024

It looks like the daily tests haven't been running for a while, i'm not sure what the issue is there. Anyway, I approved your test runs, but it fails on some module node supported version. This is probably not related to your change. To be honest, no one is actively maintaining this repository. I'm not sure how to help with that, but if you are able to fix it, it would be amazing. Otherwise, it would probably be left hanging there.

Thanks, i did changes that should fix build and tests, some of which might not be in the scope of this PR (e.g bumping some deps)

chore: pkg lock

refactor: move tests around

refactor: move nested tests around

Update yarn.lock

fix: properly terminate instrumentations

temrinal intrumentation properly

Update EntityManager.spec.ts

Update QueryBuilder.spec.ts

proper instrumentation registration

Create QueryBuilder.spec.ts

Update Repository.spec.ts
fix: make typeorm tests concurrency safe
@ragrag ragrag closed this Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants