Skip to content

Conversation

@tbpg
Copy link
Contributor

@tbpg tbpg commented Oct 11, 2018

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 11, 2018
@tbpg tbpg requested a review from rakyll October 11, 2018 19:42
Copy link
Contributor

@rakyll rakyll left a comment

Choose a reason for hiding this comment

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

Left some comments, otherwise LGTM.


// Register the view. It is imperative that this step exists,
// otherwise recorded metrics will be dropped and never exported.
if err := view.Register(latencyView); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Create this view inline. You don't need this view elsewhere.

if err := view.Register(&view.View{
Name: "demo/latency",
Measure: latencyMs,
Description: "The distribution of the latencies",
// Latency in buckets:
// [>=0ms, >=100ms, >=200ms, >=400ms, >=1s, >=2s, >=4s]
Aggregation: view.Distribution(0, 100, 200, 400, 1000, 2000, 4000),
}); err != nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


func main() {
if projectID == "" {
log.Fatal("The GOOGLE_CLOUD_PROJECT environment variable must be set.")
Copy link
Contributor

Choose a reason for hiding this comment

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

stackdriver.NewExporter resolves the project ID from the ADC, this env var is not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

// for more details.
exporter, err := stackdriver.NewExporter(stackdriver.Options{
ProjectID: projectID,
MetricPrefix: "my-metrics",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid this prefix, metric prefix is not something most users will need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

latencyMs = stats.Float64("demo/latency", "The latency in milliseconds", "ms")

latencyView = &view.View{
Name: "demo/latency",
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of the metric should represent what latency this is? Request latency, task latency, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

@tbpg tbpg left a comment

Choose a reason for hiding this comment

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

Thanks!


func main() {
if projectID == "" {
log.Fatal("The GOOGLE_CLOUD_PROJECT environment variable must be set.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


// Register the view. It is imperative that this step exists,
// otherwise recorded metrics will be dropped and never exported.
if err := view.Register(latencyView); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

// for more details.
exporter, err := stackdriver.NewExporter(stackdriver.Options{
ProjectID: projectID,
MetricPrefix: "my-metrics",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

latencyMs = stats.Float64("demo/latency", "The latency in milliseconds", "ms")

latencyView = &view.View{
Name: "demo/latency",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants