codahale/hdrhistogram github repository is archived by the owner

[tbarbugli]

I just noticed this while looking at the dependencies added by this lib:

[yurishkuro]

We shouldn't have it as a direct dependency

[jpkrohling]

This could be labeled as "good first issue".

[pohly]

@yurishkuro, @jpkrohling: what solution do you have in mind? Copy just the necessary code from codahale/hdrhistogram into jaeger-lib/metrics (which is where it is used), or copy the entire repo and continue maintaining it - but where and how?

[jpkrohling]

Isn't it just a matter of removing the dependency from glide.lock? I think it was removed from the YAML but not from the lock...

@pohly, would you be willing to give it a shot?

[pohly]

@jpkrohling no, the package still gets used here:
https://github.com/jaegertracing/jaeger-lib/blob/master/metrics/local.go#L23

So the solution isn't just a simple change to glide.lock.

[jpkrohling]

Sorry, I thought this was the main repo :-)

[pohly]

@jpkrohling do you still think that this is a "good first issue"?

codahale/hdrhistogram has some unfixed issues open, so whoever does something probably also needs to have a good understanding of whether those bugs are relevant when copying code. Doesn't look trivial to me.

I'd also like to add that this is a show-stopper for me for using Jaeger - depending on unmaintained, potentially buggy components just isn't good and won't pass a closer review.

[jpkrohling]

do you still think that this is a "good first issue"?

For someone who knows Go, I think this might still be a good first issue.

codahale/hdrhistogram has some unfixed issues open, so whoever does something probably also needs to have a good understanding of whether those bugs are relevant when copying code. Doesn't look trivial to me.

@yurishkuro can provide more details about this, but I think it would be acceptable to switch to a more modern backend, like Prometheus. Pretty much all providers nowadays are able to scrape Prometheus data. For the main backend, we are using expvar, Prometheus and a noop implementation:

https://github.com/jaegertracing/jaeger/blob/master/pkg/metrics/builder.go#L52

So, the real solution is to not depend on this unmaintained library.

depending on unmaintained, potentially buggy components just isn't good and won't pass a closer review

+1, I don't think anyone would disagree with that, but different issues have different priorities to different people.

[yurishkuro]

Local backend is only used in unit tests. We can probably simulate it via Prometheus client if we don't use the default registrar (which is global and will persist across tests).

The downside will be a new Prometheus dependency.

Another alternative is to check what go-kit uses, I expect it implements similar histogram behavior on expvar, so would need a digest library.

[pohly]

If the local backend is only used for testing, why does it get pulled into production clients? That probably should be changed first. Once that's resolved, depending on an unmaintained components becomes less of a problem.

[yurishkuro]

It should be moved to a package

[yurishkuro]

As of v2.3.0 we're not using codahale (#82).