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

Question: best way to set PrometheusExporter._serializer._appendTimestamp = false #1695

Closed
1 of 2 tasks
antoniomrfranco opened this issue Nov 23, 2020 · 3 comments
Closed
1 of 2 tasks

Comments

@antoniomrfranco
Copy link
Contributor

antoniomrfranco commented Nov 23, 2020

  • This only affects the JavaScript OpenTelemetry library
  • This may affect other libraries, but I would like to get opinions here first

Hello everyone,

I'd like to set _appendTimestamp to false for a PrometheusSerializer instance that is part of a PrometheusExporter object.

I saw that PrometheusSerializer has a property named _appendTimestamp which is defined in its constructor:

constructor(prefix?: string, appendTimestamp = true) {
if (prefix) {
this._prefix = prefix + '_';
}
this._appendTimestamp = appendTimestamp;
}

Also, I saw that the only place where PrometheusSerializer is instantiated is in the constructor method of PrometheusExporter:

this._serializer = new PrometheusSerializer(this._prefix);

However, as PrometheusExporter instantiates PrometheusSerializer passing prefix only, I can't control the value of appendTimestamp.

Is there any way to set appendTimestamp to false in this case?

Thanks.

@dyladan
Copy link
Member

dyladan commented Nov 23, 2020

I think this is just a bug. If you want to take a crack at fixing it, I suggest allowing an options object to the PrometheusExporter constructor.

/cc @legendecas any chance you can take this if @antoniomrfranco doesn't have time?

@antoniomrfranco
Copy link
Contributor Author

Thanks for the prompt response, @dyladan .
I just created a PR that fixes this issue: #1697.

@legendecas
Copy link
Member

Closing as the related PR is already landed. Thank you for the work on this!

pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
…n-telemetry#1695)

- Convert the output of run commands to a string that is readable.
- No need to `docker rm ...` test containers for `npm run test:local` usage
  because the `docker run --rm ...` option will remove them when stopped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants