-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Emit JMX metrics using OTEL SDK instead of APM Agent #142133
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
Changes from 8 commits
54ced42
bb6093a
d9017af
cb4dd45
9fc95c6
2142a8f
96ebdb9
c40bed2
da89163
da01bdc
ddacd7a
41a9d9e
7a53e2b
f25e33d
e2b64bb
fdf187d
9048950
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -299,6 +299,13 @@ private static Setting<String> concreteAgentSetting(String namespace, String qua | |
| NodeScope | ||
| ); | ||
|
|
||
| public static final Setting<Boolean> TELEMETRY_OTEL_METRICS_ENABLED_SETTING = Setting.boolSetting( | ||
|
||
| TELEMETRY_SETTING_PREFIX + "otel.metrics.enabled", | ||
| false, | ||
| OperatorDynamic, | ||
| NodeScope | ||
| ); | ||
|
|
||
| public static final Setting<SecureString> TELEMETRY_SECRET_TOKEN_SETTING = SecureSetting.secureString( | ||
| TELEMETRY_SETTING_PREFIX + "secret_token", | ||
| null | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,14 +22,13 @@ | |
|
|
||
| public class APMMeterService extends AbstractLifecycleComponent { | ||
| private final APMMeterRegistry meterRegistry; | ||
|
|
||
| private final Supplier<Meter> otelMeterSupplier; | ||
| private final Supplier<Meter> noopMeterSupplier; | ||
|
|
||
| protected volatile boolean enabled; | ||
|
|
||
| public APMMeterService(Settings settings) { | ||
| this(settings, APMMeterService.otelMeter(), APMMeterService.noopMeter()); | ||
| this(settings, createOtelMeterSupplier(settings), () -> OpenTelemetry.noop().getMeter("noop")); | ||
| } | ||
|
|
||
| public APMMeterService(Settings settings, Supplier<Meter> otelMeterSupplier, Supplier<Meter> noopMeterSupplier) { | ||
|
|
@@ -40,7 +39,17 @@ public APMMeterService(boolean enabled, Supplier<Meter> otelMeterSupplier, Suppl | |
| this.enabled = enabled; | ||
| this.otelMeterSupplier = otelMeterSupplier; | ||
| this.noopMeterSupplier = noopMeterSupplier; | ||
| this.meterRegistry = new APMMeterRegistry(enabled ? createOtelMeter() : createNoopMeter()); | ||
| this.meterRegistry = new APMMeterRegistry(enabled ? otelMeterSupplier.get() : noopMeterSupplier.get()); | ||
| if (enabled && otelMeterSupplier instanceof OtelSdkMeterSupplier) { | ||
| SystemMetrics.register(meterRegistry); | ||
| } | ||
| } | ||
|
|
||
| private static Supplier<Meter> createOtelMeterSupplier(Settings settings) { | ||
| if (APMAgentSettings.TELEMETRY_OTEL_METRICS_ENABLED_SETTING.get(settings) == false) { | ||
| return () -> GlobalOpenTelemetry.get().getMeter("elasticsearch"); | ||
| } | ||
| return new OtelSdkMeterSupplier(settings); | ||
| } | ||
|
|
||
| public APMMeterRegistry getMeterRegistry() { | ||
|
|
@@ -52,41 +61,20 @@ public APMMeterRegistry getMeterRegistry() { | |
| */ | ||
| void setEnabled(boolean enabled) { | ||
| this.enabled = enabled; | ||
| if (enabled) { | ||
| meterRegistry.setProvider(createOtelMeter()); | ||
| } else { | ||
| meterRegistry.setProvider(createNoopMeter()); | ||
| } | ||
| meterRegistry.setProvider(enabled ? otelMeterSupplier.get() : noopMeterSupplier.get()); | ||
| } | ||
|
|
||
| @Override | ||
| protected void doStart() {} | ||
|
|
||
| @Override | ||
| protected void doStop() { | ||
| meterRegistry.setProvider(createNoopMeter()); | ||
| if (otelMeterSupplier instanceof OtelSdkMeterSupplier otelSdk) { | ||
|
||
| otelSdk.close(); | ||
| } | ||
| meterRegistry.setProvider(noopMeterSupplier.get()); | ||
| } | ||
|
|
||
| @Override | ||
| protected void doClose() {} | ||
|
|
||
| protected Meter createOtelMeter() { | ||
| assert this.enabled; | ||
| return otelMeterSupplier.get(); | ||
| } | ||
|
|
||
| protected Meter createNoopMeter() { | ||
| return noopMeterSupplier.get(); | ||
| } | ||
|
|
||
| protected static Supplier<Meter> noopMeter() { | ||
| return () -> OpenTelemetry.noop().getMeter("noop"); | ||
| } | ||
|
|
||
| // to be used within doPrivileged block | ||
| private static Supplier<Meter> otelMeter() { | ||
| var openTelemetry = GlobalOpenTelemetry.get(); | ||
| var meter = openTelemetry.getMeter("elasticsearch"); | ||
| return () -> meter; | ||
| } | ||
| } | ||
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.
This seems like a major bump, should we do that in a separate change to ensure it doesn't have any side effects, apart from this PR? Also, why are we using "alpha" versions?
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.
This is one of the first things I touched, thanks for highlighting, it needs more work!
For the version bumps, there's a couple of reasons:
I am happy to separate this into it's own PR to be merged before all this if it's better!