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

Expose request handler of Prometheus exporter #1872

Closed
weyert opened this issue Jan 27, 2021 · 5 comments · Fixed by #1879
Closed

Expose request handler of Prometheus exporter #1872

weyert opened this issue Jan 27, 2021 · 5 comments · Fixed by #1879
Assignees

Comments

@weyert
Copy link
Contributor

weyert commented Jan 27, 2021

Is your feature request related to a problem? Please describe.

I would like to reuse the request handler that's implemented by the Prometheus exporter so that it can be used by different server frameworks in Node.js

Describe the solution you'd like

If the currently private request handler method is exposed by the Prometheus exporter it can be used by a wide variety of server framework in Node.js, such as Express, Koa, Hapi etc.

If you want to expose the endpoint using Express you could do something like:

server.get('/metrics', (req, res) => {
    return exporter.getRequestHandler(req, res)
  })

Alternatively, if you want to expose the endpoint using Koa:

 router.get('/metrics', async (ctx) => {
    await exporter.getRequestHandler(ctx.req, ctx.res)
    ctx.respond = false
  })

Describe alternatives you've considered

N/A

Additional context

Allow to use an existing endpoint to return Prometheus metrics based on the opentelemetry prometheus exporter implementation.

@vmarchaud
Copy link
Member

For more information, someone already tried to work on this here: #1701 however we agreed on a different way of solving the issue (pretty much what you suggested) instead of what the original PR was doing.

Are you interested in contributing this @weyert or you should we add a label to say that its open for anyone to contribute ?

@weyert
Copy link
Contributor Author

weyert commented Jan 27, 2021

@vmarchaud Sure, if you like my suggestion, I would be happy to work on a PR to apply the suggestion. Might take a bit of time as I need to get used to the code base a bit more :)

@vmarchaud
Copy link
Member

@weyert Then i'll assign it to you, feel free to ping on gitter or open a draft PR if you want some help

@weyert
Copy link
Contributor Author

weyert commented Jan 27, 2021

@vmarchaud I have started working on the pull request. This library is using mocha/chai and I don't have real experience with these testing libraries mainly jest. I will come back to the unit test once I found their version of jest.fn(). Anyways this is the approach I am thinking off.

@vmarchaud
Copy link
Member

@weyert We generally use sinon for this, here is an example: https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-exporter-collector/test/node/CollectorTraceExporter.test.ts#L54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment