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

Add integration tests #19

Closed
pragmaticivan opened this issue Jun 27, 2021 · 13 comments
Closed

Add integration tests #19

pragmaticivan opened this issue Jun 27, 2021 · 13 comments
Labels
good first issue Good for newcomers

Comments

@pragmaticivan
Copy link
Owner

No description provided.

@issackr
Copy link

issackr commented Jul 22, 2021

Hey, I created PrometheusModule for metrics based on the package and I'm trying to test it but get the error:
Nest can't resolve dependencies of the MetricService (?). Please make sure that the argument OpenTelemetryMeterProvider at index [0] is available in the PrometheusModule context.

this is my module:
import { Module } from "@nestjs/common";
import { LoggerModule } from "../../logger/logger.module";
import { PrometheusService } from "./prometheus.service";
import { MetricService } from "nestjs-otel";

@module({
imports: [LoggerModule],
providers: [PrometheusService, MetricService],
exports: [PrometheusService],
})
export class PrometheusModule {}

and my testing module:
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
imports: [PrometheusModule],
providers: [
SweetLogger,
ImageResolver,
{
provide: 'METADATA_SERVICE',
useValue: Image,
},
],
}).compile();

resolver = module.get<ImageResolver>(ImageResolver);

});

but I fail to provide OpenTelemetryMeterProvider whatever I do, I'm not sure how to provide injected stuff.
Will be glad for any help!
Tnx

@pragmaticivan
Copy link
Owner Author

pragmaticivan commented Jul 22, 2021

MetricService is injected globally when you use that lib. What you are trying to do is to inject the service directly into another module without any other dependency is available.

In your main app you need OpenTelemetryModule.forRoot( to be called in your imports for all the dependencies of this service to be available.

Is there a reason you would be creating a prometheus service? MetricService should cover most of the cases, all you need is a prometheus exporter

@issackr
Copy link

issackr commented Jul 22, 2021

basically you are right but I used interceptor to meter all requests and I noticed that the counters are reset each time if not injected one by one, so I wrapped it with a module that is MetricService alike.

Regarding the error, I added OpenTelemetryModule.forRoot() to my PrometheusModule and now I get another error:
Nest can't resolve dependencies of the OpenTelemetryCoreModule

@pragmaticivan
Copy link
Owner Author

Hey @issackr could you share more on that issue? when you mean reset each time if not injected, what you mean by that? It should reuse the existing one.

@issackr
Copy link

issackr commented Jul 25, 2021

@pragmaticivan Sure, it means the counters value aggregators remain on 1 after each call, no matter how many calls you make. Regarding the injection I meant like in other libraries which you need to inject each counter your'e using (not generic enough for me), anyway I solved it by wrapping my metric in another map of my own like in MetricService.
I can share the code if you like.
Anyway, do you have any lead how to solve my tests problem about OpenTelemetryCoreModule ModuleRef?
The error:
Nest can't resolve dependencies of the OpenTelemetryCoreModule (OpenTelemetryModuleOptions, ?). Please make sure that the argument ModuleRef at index [1] is available in the OpenTelemetryCoreModule context.

Tnx

@issackr
Copy link

issackr commented Jul 25, 2021

@pragmaticivan I also migrated my project to nest 8, maybe this is whats making problems

@pragmaticivan
Copy link
Owner Author

Ohh I haven't tested with nest 8, there might be a problem there indeed.

@pragmaticivan
Copy link
Owner Author

basically you are right but I used interceptor to meter all requests and I noticed that the counters are reset each time if not injected one by one, so I wrapped it with a module that is MetricService alike.

Regarding the error, I added OpenTelemetryModule.forRoot() to my PrometheusModule and now I get another error:
Nest can't resolve dependencies of the OpenTelemetryCoreModule

I think I found the bug you mentioned while adding more tests today.

#52

@pragmaticivan
Copy link
Owner Author

release 1.8.2 should have the fix @issackr

@issackr
Copy link

issackr commented Jul 26, 2021

@pragmaticivan Looks awesome, thanks for the great work! still having the testing ModuleRef issue though

@pragmaticivan
Copy link
Owner Author

@issackr I just fixed the reuse of metrics. I haven't tested on NestJS 8.x yet. They have changed a lot of things. Might need to test a new example this week.

@pragmaticivan
Copy link
Owner Author

fyi @issackr a lot of the problems I found when I created that lib are going to be fixed in this PR: #55

It will require a Major version though.

@issackr
Copy link

issackr commented Jul 28, 2021

@pragmaticivan Tnx for the update, I'm definitely gonna use it

@pragmaticivan pragmaticivan added the good first issue Good for newcomers label Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants