Skip to content
This repository was archived by the owner on May 27, 2026. It is now read-only.

fix the compilation error against prometheus/client-golang master#16

Merged
crosbymichael merged 1 commit into
docker:masterfrom
tifayuki:compilation_error
Jan 23, 2018
Merged

fix the compilation error against prometheus/client-golang master#16
crosbymichael merged 1 commit into
docker:masterfrom
tifayuki:compilation_error

Conversation

@tifayuki

Copy link
Copy Markdown
Contributor

This fixes the issue: #12
Signed-off-by: tifayuki [email protected]

@stevvooe

Copy link
Copy Markdown
Contributor

This fix doesn't look quite right. What was the change in prometheus that caused this compilation error?

The issue here is that prometheus.HistogramVec is no longer guaranteed. I think you need to change the field type to the observer interface.

@tifayuki

Copy link
Copy Markdown
Contributor Author

@stevvooe
Have changed to type assertion to interface observer.

Once this PR and #15 get merged, we can use it to add prometheus metrics to the docker/distribution

@tifayuki
tifayuki force-pushed the compilation_error branch 2 times, most recently from d18b090 to c04d9b1 Compare December 6, 2017 00:18
@tifayuki

tifayuki commented Dec 6, 2017

Copy link
Copy Markdown
Contributor Author

@stevvooe
Do you have time to review this PR?
Thank you

@sofuture

Copy link
Copy Markdown

Any update on this? Would love to see distribution/distribution#2466 happen

@tifayuki

Copy link
Copy Markdown
Contributor Author

@stevvooe

Can we merge these PRs?
#15
#16
distribution/distribution#2466

@nim-nim

nim-nim commented Jan 19, 2018

Copy link
Copy Markdown

Fixing compat with current prometheus would be mightily nice!

@crosbymichael

Copy link
Copy Markdown
Contributor

LGTM

@crosbymichael

Copy link
Copy Markdown
Contributor

ping @stevvooe

@crosbymichael
crosbymichael merged commit 66439bf into docker:master Jan 23, 2018
@stevvooe

Copy link
Copy Markdown
Contributor

This fix wasn't quite right. We should not be exporting an un-exported type. Either the new methods need to be added to the interface or it needs to return a different type.

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Jan 16, 2020
bump docker/go-metrics v0.0.1:

full diff: docker/go-metrics@d466d4f...v0.0.1

- docker/go-metrics#16 fix the compilation error against prometheus/client-golang master
- fixes docker/go-metrics#12 No longer builds against Prom master
- docker/go-metrics#18 metrics: address compile error correctly
- fixes docker/go-metrics#12 No longer builds against Prom master
- docker/go-metrics#15 Add functions that instruments http handler using promhttp
- docker/go-metrics#20 Rename LICENSE.code → LICENSE
- docker/go-metrics#22 Support Go Modules

bump prometheus/client_golang v0.9.4:

full diff: prometheus/client_golang@c5b7fcc...v0.9.4

version v0.9.0 is the minimum required version to work with go-metrics v0.0.1,
as it depends on `prometheus.Observer`:

    vendor/github.com/docker/go-metrics/timer.go:39:4: undefined: prometheus.Observer

Signed-off-by: Sebastiaan van Stijn <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Jan 17, 2020
bump docker/go-metrics v0.0.1:

full diff: docker/go-metrics@d466d4f...v0.0.1

- docker/go-metrics#16 fix the compilation error against prometheus/client-golang master
- fixes docker/go-metrics#12 No longer builds against Prom master
- docker/go-metrics#18 metrics: address compile error correctly
- fixes docker/go-metrics#12 No longer builds against Prom master
- docker/go-metrics#15 Add functions that instruments http handler using promhttp
- docker/go-metrics#20 Rename LICENSE.code → LICENSE
- docker/go-metrics#22 Support Go Modules

bump prometheus/client_golang v0.9.4:

full diff: prometheus/client_golang@c5b7fcc...v0.9.4

version v0.9.0 is the minimum required version to work with go-metrics v0.0.1,
as it depends on `prometheus.Observer`:

    vendor/github.com/docker/go-metrics/timer.go:39:4: undefined: prometheus.Observer

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Upstream-commit: b2db7c8bc967b52609877f5d856c338ea51e4c46
Component: engine
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants