willsoto / nestjs-prometheus

NestJS module for Prometheus
Apache License 2.0
499 stars 28 forks source link

While using .registerAsync with custom controller, custom controller not working and path is ignored #2279

Closed murbanowicz closed 3 months ago

murbanowicz commented 4 months ago

I found this really weird issue. When using useFactory / registerAsync, the custom controller does not work properly.

I have to use configuration, so I moved to async creation and I hit this issue which I cannot even workaround :/

If custom controller is passed to the options while using registerAsync, the controller is not used, but the base prom controller is being used and on top of that the path is being ignored completely.

willsoto commented 4 months ago

Can you provide a minimal reproduction please?

murbanowicz commented 4 months ago

Yes please. Simple test showcasing the issue. https://github.com/murbanowicz/nestjs-prometheus/blob/register-async-controller-issue/test/custom-controller.spec.ts

I went through the code but did not find anything obvious :/

willsoto commented 4 months ago

Thanks. I will take a look hopefully later today or tomorrow

murbanowicz commented 4 months ago

Thanks! That you for such a quick reaction.

willsoto commented 4 months ago

Out of curiosity, do you need to actually inject anything into the controller via the useFactory?

murbanowicz commented 4 months ago

I need to inject config to set custom labels. I could workaround it but it is still a bug (which I can't figure out so far while looking through the code :/

willsoto commented 4 months ago

Just to be clear, you are trying to inject something into the controller itself?

The whole reason the "custom controller" feature was added was to support Fastify. There really isn't any logic in that controller. Setting custom labels you can do via configuration only.

It's a mistake that I allow it to go in the main options, it should only exist outside the useFactory, etc. Like so:

PrometheusModule.registerAsync({
  controller: CustomController,
  useFactory() {
    // whatever goes here
    return {};
  },
});
willsoto commented 3 months ago

@murbanowicz Is this issue resolved for you?

willsoto commented 3 months ago

:tada: This issue has been resolved in version 6.0.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: