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

docs: Update stackdriver examples for tracing and stats #613

Merged
merged 2 commits into from Feb 5, 2021

Conversation

mutianf
Copy link
Contributor

@mutianf mutianf commented Feb 4, 2021

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

@mutianf mutianf requested review from as code owners Feb 4, 2021
@google-cla google-cla bot added the cla: yes label Feb 4, 2021
@codecov
Copy link

@codecov codecov bot commented Feb 4, 2021

Codecov Report

Merging #613 (b8d4a4b) into master (4893b16) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #613   +/-   ##
=========================================
  Coverage     81.71%   81.71%           
  Complexity     1156     1156           
=========================================
  Files           110      110           
  Lines          7214     7214           
  Branches        377      377           
=========================================
  Hits           5895     5895           
  Misses         1119     1119           
  Partials        200      200           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4893b16...0f4e3b3. Read the comment docs.

@product-auto-label product-auto-label bot added the api: bigtable label Feb 5, 2021
README.md Outdated
[Metrics explorer](https://console.cloud.google.com/monitoring/metrics-explorer)
page.

You can configure how frequent metrics are pushed to StackDriver and the

Choose a reason for hiding this comment

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

typo: frequently

README.md Outdated
page.

You can configure how frequent metrics are pushed to StackDriver and the
[Monitor Resource](https://cloud.google.com/monitoring/api/resources) by

Choose a reason for hiding this comment

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

better phrasing: Monitored Resource

README.md Outdated
[Metrics explorer](https://console.cloud.google.com/monitoring/metrics-explorer)
page.

You can configure how frequent metrics are pushed to StackDriver and the

Choose a reason for hiding this comment

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

nit: may be nicer to inline these suggestions into the example below. People may read code more easily than documentation

esp. for monitored resource, we want to encourage users to choose their own instead of just going w/ global.

README.md Outdated
updating the stats configuration:

``` java
// set export interval to 10 seconds and monitor resource to global

Choose a reason for hiding this comment

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

nit: Prefix this comment w/ Example, just to make it clear this isn't the standard way to do it.

Copy link
Contributor

@billyjacobson billyjacobson Feb 5, 2021

Choose a reason for hiding this comment

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

we probably don't need the specified monitored resource too. Can we link to the Java doc for this so people can easily see all the options?

Copy link
Contributor

@billyjacobson billyjacobson left a comment

Part of my confusion was around OpenCensus Tracing vs OpenCensus Stats. Perhaps we can differentiate them better in their section names and descriptions

For tracing: Client request tracing: OpenCensus Tracing
For stats: Enabling Cloud Bigtable Metrics: OpenCensus Stats

@@ -376,6 +378,28 @@ BigtableDataSettings.enableOpenCensusStats();
BigtableDataSettings.enableGfeOpenCensusStats();
Copy link
Contributor

@billyjacobson billyjacobson Feb 5, 2021

Choose a reason for hiding this comment

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

I"m not sure that this line works still. Or at least it didn't compile when I tried using it I think

Copy link
Contributor Author

@mutianf mutianf Feb 5, 2021

Choose a reason for hiding this comment

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

Ah.. Looks like this change is not released yet

README.md Outdated
updating the stats configuration:

``` java
// set export interval to 10 seconds and monitor resource to global
Copy link
Contributor

@billyjacobson billyjacobson Feb 5, 2021

Choose a reason for hiding this comment

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

we probably don't need the specified monitored resource too. Can we link to the Java doc for this so people can easily see all the options?

@shantstepanian
Copy link

@shantstepanian shantstepanian commented Feb 5, 2021

LGTM from my side; I'll defer to Billy's review on the code aspects

@billyjacobson billyjacobson merged commit 3e8af74 into googleapis:master Feb 5, 2021
16 checks passed
@mutianf mutianf deleted the update-readme branch Feb 5, 2021
gcf-merge-on-green bot pushed a commit that referenced this issue Feb 8, 2021
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.20.0](https://www.github.com/googleapis/java-bigtable/compare/v1.19.2...v1.20.0) (2021-02-05)


### Features

* Surface the server-timing metric ([#535](https://www.github.com/googleapis/java-bigtable/issues/535)) ([8240779](https://www.github.com/googleapis/java-bigtable/commit/8240779434a602dc8b2bf90dbe539c5d7470d850))


### Bug Fixes

* fix MetricTracerTest to rebase on head ([#581](https://www.github.com/googleapis/java-bigtable/issues/581)) ([23e97cb](https://www.github.com/googleapis/java-bigtable/commit/23e97cb308403b35fbe972b08048d0e59423e694))
* fix MutateRowsAttemptCallable to avoid NPE in MetricTracer ([#557](https://www.github.com/googleapis/java-bigtable/issues/557)) ([8d71020](https://www.github.com/googleapis/java-bigtable/commit/8d7102003b54757b64fd598290301d3b24fd9c29))
* Retry "received rst stream" ([#586](https://www.github.com/googleapis/java-bigtable/issues/586)) ([b09a21c](https://www.github.com/googleapis/java-bigtable/commit/b09a21c1dd1a05b68bfd3a0134089ba32dca1774))
* update repo name ([#615](https://www.github.com/googleapis/java-bigtable/issues/615)) ([bb3ed6d](https://www.github.com/googleapis/java-bigtable/commit/bb3ed6dcbadbd70dbd9c68152c8d93c4cefd4dcb))


### Dependencies

* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.17.1 ([#590](https://www.github.com/googleapis/java-bigtable/issues/590)) ([5035ad0](https://www.github.com/googleapis/java-bigtable/commit/5035ad0db01a9247634137050698c30da29722a6))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.18.0 ([#592](https://www.github.com/googleapis/java-bigtable/issues/592)) ([c58b73a](https://www.github.com/googleapis/java-bigtable/commit/c58b73a7d70c8da1581ac06d77b5e362648a0868))
* update dependency com.google.errorprone:error_prone_annotations to v2.5.0 ([#591](https://www.github.com/googleapis/java-bigtable/issues/591)) ([dfa4da7](https://www.github.com/googleapis/java-bigtable/commit/dfa4da75e5ac81cc941177462326f7c38f18bacd))
* update dependency com.google.errorprone:error_prone_annotations to v2.5.1 ([#594](https://www.github.com/googleapis/java-bigtable/issues/594)) ([ea599a1](https://www.github.com/googleapis/java-bigtable/commit/ea599a10e2e4fdbaf56c45b74fbb1ea5a708a7f2))


### Documentation

* Expand hello world snippet to show how to access specific cells ([#516](https://www.github.com/googleapis/java-bigtable/issues/516)) ([a9001a8](https://www.github.com/googleapis/java-bigtable/commit/a9001a88f338fc2acf6bc48927765f29819124ee))
* Update stackdriver examples for tracing and stats ([#613](https://www.github.com/googleapis/java-bigtable/issues/613)) ([3e8af74](https://www.github.com/googleapis/java-bigtable/commit/3e8af747b329f6656a410160e8da14fd8227c8fc))
* use autogenerated readme functionality and regenerate ([#568](https://www.github.com/googleapis/java-bigtable/issues/568)) ([844e5be](https://www.github.com/googleapis/java-bigtable/commit/844e5beb6230df6ca220935056aae7f6e73d2bc0))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
ad548 pushed a commit to ad548/java-bigtable that referenced this issue Mar 13, 2021
* docs: Update stackdriver examples for tracing and stats

* update based on review
ad548 pushed a commit to ad548/java-bigtable that referenced this issue Mar 13, 2021
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.20.0](https://www.github.com/googleapis/java-bigtable/compare/v1.19.2...v1.20.0) (2021-02-05)


### Features

* Surface the server-timing metric ([googleapis#535](https://www.github.com/googleapis/java-bigtable/issues/535)) ([8240779](https://www.github.com/googleapis/java-bigtable/commit/8240779434a602dc8b2bf90dbe539c5d7470d850))


### Bug Fixes

* fix MetricTracerTest to rebase on head ([googleapis#581](https://www.github.com/googleapis/java-bigtable/issues/581)) ([23e97cb](https://www.github.com/googleapis/java-bigtable/commit/23e97cb308403b35fbe972b08048d0e59423e694))
* fix MutateRowsAttemptCallable to avoid NPE in MetricTracer ([googleapis#557](https://www.github.com/googleapis/java-bigtable/issues/557)) ([8d71020](https://www.github.com/googleapis/java-bigtable/commit/8d7102003b54757b64fd598290301d3b24fd9c29))
* Retry "received rst stream" ([googleapis#586](https://www.github.com/googleapis/java-bigtable/issues/586)) ([b09a21c](https://www.github.com/googleapis/java-bigtable/commit/b09a21c1dd1a05b68bfd3a0134089ba32dca1774))
* update repo name ([googleapis#615](https://www.github.com/googleapis/java-bigtable/issues/615)) ([bb3ed6d](https://www.github.com/googleapis/java-bigtable/commit/bb3ed6dcbadbd70dbd9c68152c8d93c4cefd4dcb))


### Dependencies

* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.17.1 ([googleapis#590](https://www.github.com/googleapis/java-bigtable/issues/590)) ([5035ad0](https://www.github.com/googleapis/java-bigtable/commit/5035ad0db01a9247634137050698c30da29722a6))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.18.0 ([googleapis#592](https://www.github.com/googleapis/java-bigtable/issues/592)) ([c58b73a](https://www.github.com/googleapis/java-bigtable/commit/c58b73a7d70c8da1581ac06d77b5e362648a0868))
* update dependency com.google.errorprone:error_prone_annotations to v2.5.0 ([googleapis#591](https://www.github.com/googleapis/java-bigtable/issues/591)) ([dfa4da7](https://www.github.com/googleapis/java-bigtable/commit/dfa4da75e5ac81cc941177462326f7c38f18bacd))
* update dependency com.google.errorprone:error_prone_annotations to v2.5.1 ([googleapis#594](https://www.github.com/googleapis/java-bigtable/issues/594)) ([ea599a1](https://www.github.com/googleapis/java-bigtable/commit/ea599a10e2e4fdbaf56c45b74fbb1ea5a708a7f2))


### Documentation

* Expand hello world snippet to show how to access specific cells ([googleapis#516](https://www.github.com/googleapis/java-bigtable/issues/516)) ([a9001a8](https://www.github.com/googleapis/java-bigtable/commit/a9001a88f338fc2acf6bc48927765f29819124ee))
* Update stackdriver examples for tracing and stats ([googleapis#613](https://www.github.com/googleapis/java-bigtable/issues/613)) ([3e8af74](https://www.github.com/googleapis/java-bigtable/commit/3e8af747b329f6656a410160e8da14fd8227c8fc))
* use autogenerated readme functionality and regenerate ([googleapis#568](https://www.github.com/googleapis/java-bigtable/issues/568)) ([844e5be](https://www.github.com/googleapis/java-bigtable/commit/844e5beb6230df6ca220935056aae7f6e73d2bc0))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants