Metering API Implementation #99723
Closed
Metering API Implementation #99723
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
…ticsearch into otel-meter-api-integ
…search into stu-elastic-otel-meter-api
with the support of metrics the TracerPlugin name is no longer adequate. Renaming this to TelemetryPlugin. Also introducing TelemetryProvider interface. While it is only used in Node.java at the moment to fetch Tracer instance, it is intended to be used in Plugin::createComponents (to be done in separate commite due to the broad scope of this method) This will allow for plugins to get access to both Tracer and Metric interfaces without the need to add yet another argument to createComponents Also adding internal subpackage in module/apm so that it is more obvious which packages are not exported
update with latest main (package renames)
rjernst
reviewed
Sep 21, 2023
| @@ -82,7 +84,7 @@ class APMJvmOptions { | |||
| // is doing, leave this value alone. | |||
| "log_level", "error", | |||
| "application_packages", "org.elasticsearch,org.apache.lucene", | |||
| "metrics_interval", "120s", | |||
| "metrics_interval", "5s", | |||
There was a problem hiding this comment.
5 seconds seems pretty aggressive. Do we really need metric resolution at that granularity?
modules/apm/build.gradle
Outdated
| dependencies { | ||
| implementation "io.opentelemetry:opentelemetry-api:${otelVersion}" | ||
| implementation "io.opentelemetry:opentelemetry-context:${otelVersion}" | ||
| implementation "io.opentelemetry:opentelemetry-semconv:${otelVersion}-alpha" | ||
| runtimeOnly "co.elastic.apm:elastic-apm-agent:1.36.0" | ||
| implementation "co.elastic.apm:elastic-apm-agent:1.42.1-SNAPSHOT" |
There was a problem hiding this comment.
We should not commit a snapshot dependency, we'll need to wait until this is released.
| AllocationService allocationService, | ||
| IndicesService indicesService | ||
| ) { | ||
| final APMTracer apmTracer = tracer.get(); | ||
| final APMTracer apmTracer = telemetryProvider.get().getTracer(); |
There was a problem hiding this comment.
Why is the provider captured, when it is already passed in? It should be the exact same, so we could avoid the SetOnce?
| @@ -50,10 +50,12 @@ class APMJvmOptions { | |||
| // by the agent. Don't disable writing to a log file, as the agent will then | |||
| // require extra Security Manager permissions when it tries to do something | |||
| // else, and it's just painful. | |||
| "log_file", "_AGENT_HOME_/../../logs/apm.log", | |||
|
|
|||
| "log_file", "/Users/przemyslawgomulka/workspace/pgomulka/apm.log", | |||
There was a problem hiding this comment.
The log file and level need to be reset back to their original levels.
|
Superseded by #99832 |
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.
This pulls in @pgomulka's branch stu-elastic-otel-meter-api and adds the elasticsearch versions of the otel instruments.