-
Notifications
You must be signed in to change notification settings - Fork 66
Add Application Signals runtime metrics #881
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 Application Signals runtime metrics #881
Conversation
| List<String> jmxTargets = new ArrayList<>(list); | ||
| jmxTargets.add("jvm"); | ||
| Map<String, String> propsOverride = new HashMap<>(1); | ||
| propsOverride.put(OTEL_JMX_TARGET_SYSTEM_CONFIG, String.join(",", jmxTargets)); |
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.
q: with AppSignals enabled, if customer sets OTEL_JMX_TARGET_SYSTEM_CONFIG with value such as kafka , what will be AppSignals behavior? will we OTel Gatherer also collector kafka metrics but dropped in CWAgent eventually?
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.
Yes. They will be dropped.
Metrics are not exported directly in CWA. The metrics will be further transformed in CWA, and some of them will be added a new attribute Telemetry.Source to be selectable by EMF exporter. Metrics not declared in the transformation rules will be dropped in the end.
| if (exporterNames.contains("none")) { | ||
| for (String scope : registeredScopeNames) { | ||
| sdkMeterProviderBuilder.registerView( | ||
| InstrumentSelector.builder().setMeterName(scope).build(), | ||
| View.builder().setAggregation(Aggregation.defaultAggregation()).build()); | ||
|
|
||
| logger.log(Level.FINE, "Registered scope {0}", scope); | ||
| } | ||
| sdkMeterProviderBuilder.registerView( | ||
| InstrumentSelector.builder().setName("*").build(), | ||
| View.builder().setAggregation(Aggregation.drop()).build()); | ||
| } |
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.
could you help to add some comment on this method? I am not sure if I get the intention correctly?
Does it mean that - If customer set metric_exporter to none, we will only keep the configured runtime metrics and drop everything else? Otherwise, we keep everything?
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.
View is registered when metric_exporter to none so that only runtime metrics are collected through MeterProvider. In other cases, we respect the original metric collection rules.
| /** | ||
| * The {@code ScopeBasedPeriodicMetricReader} class is a customized implementation to extend the | ||
| * functionality of the {@link io.opentelemetry.sdk.metrics.export.PeriodicMetricReader}. Due to the | ||
| * fact that {@link io.opentelemetry.sdk.metrics.export.PeriodicMetricReader} is a final class and |
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.
nit: add comments about the version (commit hash?) of original file as baseline.
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.
Added the corresponding OTEL version.
vastin
left a comment
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.
LGTM
253dddb to
9610378
Compare
9610378 to
5d2719a
Compare
|
Closed this PR and track the merge process in #892. |
## Feature request Add runtime metrics collection into Application Signals. ## Description of changes: This PR is (mostly) equivalent to [Add Application Signals runtime metrics #881](#881). The difference is that it disables runtime metrics for now. We split the core function from feature enablement because there are other ongoing feature developments depending on the changes made by runtime metrics. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. *Issue #, if available:* *Description of changes:* By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Feature request Add runtime metrics collection into Application Signals. ## Description of changes: 1. Add JMX metrics template. 1. The JMX template is taken from aws-observability#817. The template is updated with bug fixes and added with additional CPU metrics. The author of the original PR is notified with the changes. 2. Introduce `CloudWatchTemporalitySelector` to set temporality of Application Signals exporter to `alwaysDelta`. 3. Add `ScopeBasedPeriodicMetricReader` to copy metrics from `io.opentelemetry.jmx` instrumentation scope to Application Signals exporter. 4. Set `aws.local.service` into resource attributes. ## Result example ``` { resource=Resource{ schemaUrl=https: //opentelemetry.io/schemas/1.21.0, attributes={ aws.local.service="ec2-runtime-demo", cloud.account.id="633750930120", cloud.availability_zone="us-east-1a", cloud.platform="aws_ec2", cloud.provider="aws", cloud.region="us-east-1", host.arch="amd64", host.id="i-03ff80a878a803e0e", host.image.id="ami-0ae8f15ae66fe8cda", host.name="ip-172-31-25-215.ec2.internal", host.type="t2.medium", os.description="Linux 6.1.102-108.177.amzn2023.x86_64", os.type="linux", process.command_args=[ /usr/lib/jvm/java-21-amazon-corretto.x86_64/bin/java, -Dotel.instrumentation.micrometer.enabled=false, -jar, spring-petclinic-all-in-one.jar ], process.executable.path="/usr/lib/jvm/java-21-amazon-corretto.x86_64/bin/java", process.pid=445291, process.runtime.description="Amazon.com Inc. OpenJDK 64-Bit Server VM 21.0.4+7-LTS", process.runtime.name="OpenJDK Runtime Environment", process.runtime.version="21.0.4+7-LTS", service.name="ec2-runtime-demo", service.version="3.1.0-SNAPSHOT", telemetry.auto.version="1.33.0-aws-SNAPSHOT", telemetry.sdk.language="java", telemetry.sdk.name="opentelemetry", telemetry.sdk.version="1.34.1" } }, instrumentationScopeInfo=InstrumentationScopeInfo{ name=io.opentelemetry.jmx, version=null, schemaUrl=null, attributes={ } }, name=jvm.gc.collections.elapsed, description=Theapproximateaccumulatedcollectionelapsedtimeinmilliseconds, unit=ms, type=LONG_SUM, data=ImmutableSumData{ points=[ ImmutableLongPointData{ startEpochNanos=1724126549515601332, epochNanos=1724126609522082935, attributes={ name="G1 Old Generation" }, value=0, exemplars=[ ] }, ImmutableLongPointData{ startEpochNanos=1724126549515601332, epochNanos=1724126609522082935, attributes={ name="G1 Concurrent GC" }, value=49, exemplars=[ ] }, ImmutableLongPointData{ startEpochNanos=1724126549515601332, epochNanos=1724126609522082935, attributes={ name="G1 Young Generation" }, value=332, exemplars=[ ] } ], monotonic=true, aggregationTemporality=CUMULATIVE } } ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Feature request
Add runtime metrics collection into Application Signals.
Description of changes:
CloudWatchTemporalitySelectorto set temporality of Application Signals exporter toalwaysDelta.ScopeBasedPeriodicMetricReaderto copy metrics fromio.opentelemetry.jmxinstrumentation scope to Application Signals exporter.aws.local.serviceinto resource attributes.Result example
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.