Skip to content
This repository was archived by the owner on Apr 1, 2025. It is now read-only.

support serving up go-metrics as expvars #96

Merged
merged 3 commits into from
Jan 13, 2016
Merged

Conversation

Dieterbe
Copy link
Contributor

not thoroughly tested yet, but seems to work fine so far.

// panic: http: multiple registrations for /debug/vars
// http.HandleFunc("/debug/vars", e.expHandler)
// haven't found an elegant way, so just use a different endpoint
http.HandleFunc("/debug/vars2", e.expHandler)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make this endpoint configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what would be the point of introducing variability for this?
it makes more sense to me IMHO to have some sort of "convention" for the address of a "go-metrics enabled expvar page". one thing is for sure: I couldn't find a way to take over/use
/debug/vars. Actually now that I think about it, /debug/vars is also fixed for convention, which I think is a good thing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a good point. Then how about just 'debug/metrics'? I'm generally not
a fan of adding numbers to variable names and urls where there's an
opportunity to have a meaningful name.

On Saturday, July 4, 2015, Dieter Plaetinck [email protected]
wrote:

In exp/exp.go
#96 (comment):

  •   if !first {
    
  •       fmt.Fprintf(w, ",\n")
    
  •   }
    
  •   first = false
    
  •   fmt.Fprintf(w, "%q: %s", kv.Key, kv.Value)
    
  • })
  • fmt.Fprintf(w, "\n}\n")
    +}

+func Exp(r metrics.Registry) {

  • e := exp{sync.Mutex{}, r}
  • // this would cause a panic:
  • // panic: http: multiple registrations for /debug/vars
  • // http.HandleFunc("/debug/vars", e.expHandler)
  • // haven't found an elegant way, so just use a different endpoint
  • http.HandleFunc("/debug/vars2", e.expHandler)

what would be the point of introducing variability for this?
it makes more sense to me IMHO to have some sort of "convention" for the
address of a "go-metrics enabled expvar page". one thing is for sure: I
couldn't find a way to take over/use
/debug/vars. Actually now that I think about it, /debug/vars is also
fixed for convention, which I think is a good thing.


Reply to this email directly or view it on GitHub
https://github.com/rcrowley/go-metrics/pull/96/files#r33890766.

Sent from Russia, with love. But also from my phone.

@mihasya mihasya added the feature label Jul 4, 2015
@sargun
Copy link

sargun commented Nov 14, 2015

Any reason why this didn't get merged?

@mihasya
Copy link
Collaborator

mihasya commented Nov 14, 2015

Yes, but no relevant ones. Everyone involved in maintaining this repo has
had a pretty intense month or two (acquisitions, babies, etc), so reviews
have not been timely. Hoping to get caught up tomorrow.

On Friday, November 13, 2015, Sargun Dhillon [email protected]
wrote:

Any reason why this didn't get merged?


Reply to this email directly or view it on GitHub
#96 (comment).

Sent from Russia, with love. But also from my phone.

@Dieterbe
Copy link
Contributor Author

sounds like very good reasons to me. congrats to the babies and acquisitions everyone ;)

@mihasya
Copy link
Collaborator

mihasya commented Nov 29, 2015

OK finally getting back to this. I still would like to see a URL that was something more descriptive than vars2 - /debug/metrics or something like that. Variables with numbers at the end are a code smell, and IMO so are URLs. Otherwise, this is good to go. Thank you for your patience.

@Dieterbe
Copy link
Contributor Author

there you go ^^

@ghost
Copy link

ghost commented Jan 6, 2016

Is there anything I can do to help with in order to get this merged?

@dzlab
Copy link

dzlab commented Jan 13, 2016

why this is not yet on master??

@mihasya
Copy link
Collaborator

mihasya commented Jan 13, 2016

I keep forgetting to click merge. At the office in ~10, will click.

On Wednesday, January 13, 2016, El-Soufy [email protected] wrote:

why this is not yet on master??


Reply to this email directly or view it on GitHub
#96 (comment).

Sent from Russia, with love. But also from my phone.

mihasya added a commit that referenced this pull request Jan 13, 2016
support serving up go-metrics as expvars
@mihasya mihasya merged commit 51425a2 into rcrowley:master Jan 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants