-
Notifications
You must be signed in to change notification settings - Fork 329
Conversation
|
||
[embedmd]:# (gauge.go record) | ||
```go | ||
allocEntry.Set(int64(getAlloc())) // int64 gauge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: indentation seems to be off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is auto generated.
examples/gauges/README.md
Outdated
[embedmd]:# (gauge.go alloc) | ||
```go | ||
allocGauge, err := r.AddInt64Gauge( | ||
"process/heap/alloc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Prometheus doesn't allow '/', consider replacing with '_' instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prometheus exporter replaces '/' with ''.
Should we '/' or '' be based on exporter? or based on what is more appropriate generically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this example I'm inclined to be consistent with the exporter, since it will be confusing if users see the '/' characters being swapped on Prometheus endpoint. In addition the readme suggests the metric names use '_' ("process_heap_alloc"): https://github.com/census-instrumentation/opencensus-go/pull/1107/files#diff-2b3521dd2010161fca962a345a4eaddeR21.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SG
* Add gauges example. * use _ instead of /
* Add gauges example. * use _ instead of /
fixes #1035 partially