Migrate to Open Telemetry #6
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Open Tracing has now migrated to the Open Telemetry project. In addition to tracing, Open Telemetry also supports metrics. By migrating to Open Telemetry, we can do away with both Open Tracing and DropWizard Metrics and support what looks to be becoming a new standard. Merging this PR will warrant a semver update to 2.0.0 as we're taking quite a new approach. Open Tracing is relatively easy to migrate (note that "logs" and now "events" on the tracing API). Metrics is a re-write, but a good one given industry support. JSON serialisation has now been replaced with marshalling to Open Telemetry's standard protobuf domain model. Instead of using `jq` and `nc` to observe services, we'll be able to use `protoc --decode-raw` and `nc` instead.
|
|
||
| override def `export`(metrics: util.Collection[MetricData]): MetricExporter.ResultCode = { | ||
| metrics.forEach { metric => | ||
| val _ = metricQueue.offer(metric) |
There was a problem hiding this comment.
Do you need to assign this val for linting purposes? looks like it could otherwise be shortened to
metrics.forEach(metricQueue.offer)
There was a problem hiding this comment.
It’s a wart to not use an assignment where a value is returned. It could indicate a mistake, hence the wart remover plugin highlighting this.
There was a problem hiding this comment.
Fair enough, I suppose it's a consequence of not caring about the return value of the Java API in this case. Perhaps worth writing an extension method wrapping this call with a Scala idiomatic Unit return type assuming the return value is really unimportant and if this was to be called in many places in the code but not worth it for one instance!
A few OTEL changes to contend with
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Open Tracing has now migrated to the Open Telemetry project. In addition to tracing, Open Telemetry also supports metrics. By migrating to Open Telemetry, we can do away with both Open Tracing and DropWizard Metrics and support what looks to be becoming a new standard.
Merging this PR will warrant a version update to 2.0.0 as we're taking quite a new approach. Open Tracing is relatively easy to migrate (note that "logs" are now "events" on the tracing API). Metrics is a re-write, but a good one, given its potential for industry support.
JSON serialization has now been replaced with marshalling to Open Telemetry's standard protobuf domain model. Instead of using
jqandncto observe services, we'll be able to useprotoc --decode-rawandncinstead.TODO: