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

update prom-client to 12.0.0 #1139

Closed

Conversation

shivkanya9146
Copy link
Contributor

@shivkanya9146 shivkanya9146 commented Jun 3, 2020

Which problem is this PR solving?

-update prom-client

Short description of the changes

-updated prom-client to 12.0.0

@dyladan dyladan changed the title update promo-client to 12.0.0 update prom-client to 12.0.0 Jun 3, 2020
Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

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

LGTM assuming tests pass

@dyladan
Copy link
Member

dyladan commented Jun 3, 2020

Looks like this was a breaking change in prom-client which will require some additional work to handle.

@dyladan dyladan self-requested a review June 3, 2020 19:57
@mayurkale22
Copy link
Member

I think build is failing due to siimon/prom-client#299, prom-client v12.0.0 introduced some breaking changes.

@mayurkale22
Copy link
Member

In siimon/prom-client#333 timestamp support is removed from the prom-client library.

@naseemkullah
Copy link
Member

fwiw labelValues will be removed via #1126

@dyladan
Copy link
Member

dyladan commented Jun 3, 2020

fwiw labelValues will be removed via #1126

No they will not. Just labelKeys.

@shivkanya9146 shivkanya9146 force-pushed the update_promo_client branch from 1170553 to ff2524e Compare June 3, 2020 21:06
@mayurkale22
Copy link
Member

fwiw labelValues will be removed via #1126

No they will not. Just labelKeys.

labelValues is removed from the import in https://github.com/open-telemetry/opentelemetry-js/pull/1126/files#diff-7803f704226aa2df8c0c4ca194a4ae9dL34, I think this is what he meant.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
@dyladan
Copy link
Member

dyladan commented Jun 3, 2020

that makes sense

@@ -140,7 +136,6 @@ export class PrometheusExporter implements MetricExporter {
metric.inc(
labelValues,
point.value as Sum,
hrTimeToMilliseconds(point.timestamp)
Copy link
Member

Choose a reason for hiding this comment

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

why is this not needed anymore ?

Copy link
Member

Choose a reason for hiding this comment

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

Version 12 of the prom client removed the timestamp siimon/prom-client#333

@naseemkullah
Copy link
Member

Hi @shivkanya9146 could you please resolve the conflicts?

@obecny
Copy link
Member

obecny commented Sep 8, 2020

prom-clienthas been removed from prometheus and serializer was introduced. This is not valid anymore, closing.

@obecny obecny closed this Sep 8, 2020
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

Successfully merging this pull request may close these issues.

None yet

5 participants