Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Quickstart/Node.js Metrics: Add Node.js Metrics Codelab #513

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PikBot
Copy link
Contributor

@PikBot PikBot commented Dec 24, 2018

Codelab for Node.js metrics quickstart

@odeke-em please review!

AggregationType.COUNT,
[methodTagKey],
"The number of lines from standard input"
)
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon (;) is missing.

// Bucket Boudaries:
// [>=0B, >=5B, >=10B, >=15B, >=20B, >=40B, >=60B, >=80, >=100B, >=200B, >=400, >=600, >=800, >=1000]
[0, 5, 10, 15, 20, 40, 60, 80, 100, 200, 400, 600, 800, 1000]
)
Copy link
Member

Choose a reason for hiding this comment

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

Same here, Semicolon (;) is missing.

const stats = new Stats();

// Add your project id to the Stackdriver options
const exporter = new StackdriverStatsExporter({projectId: "your-project-id"});
Copy link
Member

Choose a reason for hiding this comment

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

I think it is worth to mention below lines here,

// Exporters use Application Default Credentials to authenticate.
// See https://developers.google.com/identity/protocols/application-default-credentials
// for more details.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that would be useful for the reader.

stats.record({
measure: mLatencyMs,
tags,
value: (new Date()) - startTime.getTime()
Copy link
Member

Choose a reason for hiding this comment

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

This should be value: new Date().getTime() - startTime.getTime() . Also, startTime variable is not defined anywhere in the code.

@mayurkale22
Copy link
Member

Following suit with #512, we should use open source exporter (Prometheus) for Node.js codelabs.

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.

4 participants