Skip to content

Commit

Permalink
Polish description of process.start.time
Browse files Browse the repository at this point in the history
  • Loading branch information
Jon Schneider committed Jul 19, 2018
1 parent 11f2606 commit 5632cd4
Showing 1 changed file with 1 addition and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void bindTo(MeterRegistry registry) {

TimeGauge.builder("process.start.time", runtimeMXBean, TimeUnit.MILLISECONDS, RuntimeMXBean::getStartTime)
.tags(tags)
.description("Start time of the process since unix epoch in seconds.")

This comment has been minimized.

Copy link
@ofirnk

ofirnk Jul 24, 2018

This polish indeed breaks pushgateway integration as of v1.0.6 :

An error has occurred during metrics collection:

gathered metric family process_start_time_seconds has help "Start time of the process since unix epoch." but should have "Start time of the process since unix epoch in seconds."

Which results in HTTP 500 error when pulling metrics from the gateway

This comment has been minimized.

Copy link
@mweirauch

mweirauch Jul 24, 2018

Contributor

Can you please share your setup of PG and Prom (configs, versions) in a new issue and link the old one (or a PR) which should mitigate the issues you have?
Also please elaborate if this actually leads to the same issues as before 1.0.6 or if with 1.0.5 different PG pull issues have been present. (I'd assume they are identical)
Thanks!

We are using the PG just fine and I am wondering what the problem might be.

This comment has been minimized.

Copy link
@ofirnk

ofirnk via email Jul 24, 2018

This comment has been minimized.

Copy link
@mweirauch

mweirauch Jul 24, 2018

Contributor

I am regularly checking updated versions, but I apparently missed updating PushGateway. Was still using prom/pushgateway:v0.4.0 and this one doesn't exhibit the problem. Starting with prom/pushgateway:v0.5.0 and onwards the problem arises. As a quick solution, I'd propose to downgrade in the meantime. (Assume there'll be no flags changing this behaviour back in PushGateway.)

This comment has been minimized.

Copy link
@mweirauch

mweirauch Jul 24, 2018

Contributor

Something we could incorporate:

    @Bean
    MeterFilter pushgatewayCompatMeterFilter() {
        return new MeterFilter() {

            @Override
            public Id map(Id id) {
                if (!"process.start.time".equals(id.getName())) {
                    return id;
                }
                /*
                 * Starting with prom/pushgateway:v0.5.0 differing meter
                 * descriptions are not accepted anymore. Pushgateway is
                 * exposing its own "process_start_time_seconds" metric which
                 * has a differing description than the one from Micrometer.
                 */
                return new Id(id.getName(), id.getTags(), id.getBaseUnit(),
                        "Start time of the process since unix epoch in seconds.", id.getType());
            }

        };
    }

This comment has been minimized.

Copy link
@ofirnk

ofirnk via email Jul 24, 2018

This comment has been minimized.

Copy link
@izeye

izeye Jul 24, 2018

Contributor

Based on the discussion from #727 (comment), I thought that the breaking change was intentional although it's unfortunate that there was no follow-up before a release.

.description("Start time of the process since unix epoch.")
.register(registry);
}
}

1 comment on commit 5632cd4

@mweirauch
Copy link
Contributor

Choose a reason for hiding this comment

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

Tested locally. Works for me. Will put it into a PR.

Please sign in to comment.