-
Notifications
You must be signed in to change notification settings - Fork 94
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
feat: migrate to OTEL exporter #1788
Conversation
85d5237
to
1868c5d
Compare
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Outdated
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Outdated
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Outdated
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Show resolved
Hide resolved
final String jvmName = ManagementFactory.getRuntimeMXBean().getName(); | ||
// If not the expected format then generate a random number. | ||
if (jvmName.indexOf('@') < 1) { | ||
String hostname = "localhost"; | ||
try { | ||
hostname = InetAddress.getLocalHost().getHostName(); | ||
} catch (UnknownHostException e) { | ||
logger.log(Level.INFO, "Unable to get the hostname.", e); | ||
} | ||
// Generate a random number and use the same format "random_number@hostname". | ||
return "java-" + new SecureRandom().nextInt() + "@" + hostname; |
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.
why dont we use uuid always?
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.
Since this interacts with the real world, I think you need an integration test
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Show resolved
Hide resolved
|
||
private static final String RESOURCE_TYPE = "bigtable_client_raw"; | ||
|
||
private CompletableResultCode lastCode; |
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.
Can you rename this to be a more descriptive? lastExportResultCode? Also it seems like this will be accessed from different threads, so needs some synchronization
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.
I think CompletableResultCode .fail() and .success() is already synchronized
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
No region tags are edited in this PR.This comment is generated by snippet-bot.
|
Warning: This pull request is touching the following templated files:
|
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Outdated
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Outdated
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...e/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsAttributes.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
...ain/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java
Outdated
Show resolved
Hide resolved
* feat: migrate to OTEL exporter (#1788) * feat: migrate exporter to OTEL * address comments * filter out only bigtable metrics * fix test * use the bom * update * update * update completeResultCode * add a comment * address comments * address comments * update pom * small fix * also check timestamp * address comment * updates * update * do not block on shutdown * chore: refactor factory class (#2081) * chore: refactor TracerFactory creation (#2102) * feat: migrate built in metrics to OTEL (#1796) * feat: migrate exporter to OTEL * address comments * filter out only bigtable metrics * fix test * use the bom * update * update * feat: migrate builtin metrics to OTEl * update completeResultCode * add a comment * udpate * fix tests * remove unrelated changes * fix tests * add documentation * fix test * merge exporter changes * address comments * rebase on otel * revert changes in stats * fix import * update * merge back the endpoint change * refactor constants and settings * refactor and fix tests * remove unused dependency * add some javadoc * address part of the comments * update test * test with nano * measure everything in nanos and publish with double histogram * address comments * fix test * add toString * merge from main * feat: migrate per connection error count metric to otel (#2133) * feat: migrate per connection error count metric to otel * update test * address comments * remove unnecessary check * clean up statsRecorder * remove dependency * address comments * feat: migrate per connection error exporter to otel (#2152) * chore: clean up stats package (#2163) * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix clirr and integration test * fix clirr and integration test * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * log more information on test failures * address comments * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * include version * fix flaky test * remove otel dependencies since they're added to shared dependencies * backport new tests since 2.37 * set feature flag when metrics is enabled * record metrics whenever it's registered * record metrics whenever it's registered * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * remove stale reference * clean up --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Changes in this PR are independent from Bigtable. It only interacts with cloud monitoring API. So there's no rollback plan. The correctness of the metrics conversion from OTEL to cloud monitoring format are tested in
BigtableCloudMonitoringExporterTest
unit test. The end to end IT test will be added to the following PR.