Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add OpenCensus metrics quickstart #2079

Merged
merged 10 commits into from
Apr 23, 2019

Conversation

c24t
Copy link
Contributor

@c24t c24t commented Apr 2, 2019

Add quickstart docs for exporting metrics to stackdriver via the opencensus client.

See previous PRs for the same feature in go, java, and node.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 2, 2019
@c24t
Copy link
Contributor Author

c24t commented Apr 3, 2019

cc @mayurkale22

@mayurkale22
Copy link

/cc @andrewsg Please review the PR.

Copy link
Member

@andrewsg andrewsg left a comment

Choose a reason for hiding this comment

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

We'll need tests before merging, and ideally a readme. The code itself lgtm.

ms = random() * 5 * 1000
print("Latency {}: {}".format(ii, ms))

mmap = stats.stats.stats_recorder.new_measurement_map()
Copy link
Member

Choose a reason for hiding this comment

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

This is already imported as from opensensus.stats import stats, so that would make this opencensus.stats.stats.stats.stats_recorder. Is that really accurate? It's not a typo of too many stats?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a lot of stutter, but that's correct. stats 1 is the package, stats 2 is the module, stats 3 is the global Stats object.

exporter, transport = stats_exporter.new_stats_exporter()

# Record 100 fake latency values between 0 and 5 seconds.
for ii in range(100):
Copy link
Member

Choose a reason for hiding this comment

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

Why ii here instead of i? Actually, can we use a more descriptive variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to avoid single-letter names. I don't know what would be more descriptive here, possibly _ to indicate that we're not using the var for anything.

@c24t
Copy link
Contributor Author

c24t commented Apr 16, 2019

@andrewsg let me know if this is what you're looking for in the README and tests.

@andrewsg
Copy link
Member

Looks great, thanks!

@andrewsg andrewsg merged commit 3619a77 into GoogleCloudPlatform:master Apr 23, 2019
@c24t c24t deleted the oc-python-metrics branch April 23, 2019 22:20
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.

4 participants