-
Notifications
You must be signed in to change notification settings - Fork 8
implementing JFR and Open telemetry providers to monitor UCP #217
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
base: observability-provider
Are you sure you want to change the base?
implementing JFR and Open telemetry providers to monitor UCP #217
Conversation
7f305d3
to
9cb4b41
Compare
<artifactId>ojdbc</artifactId> | ||
<version>11.2.0.4</version> <!-- Change to your version --> | ||
<scope>system</scope> | ||
<systemPath>/Users/abdessamadelaaissaoui/Desktop/ojdbc11.jar</systemPath> |
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.
You should avoid adding paths to your own environment.
<artifactId>ucp</artifactId> | ||
<version>11.2.0.4</version> | ||
<scope>system</scope> | ||
<systemPath>/Users/abdessamadelaaissaoui/Desktop/ucp11.jar</systemPath> |
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.
Same here
<dependency> | ||
<groupId>io.opentelemetry</groupId> | ||
<artifactId>opentelemetry-api</artifactId> | ||
<version>1.32.0</version> |
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 are we using a different version here?
<dependency> | ||
<groupId>io.opentelemetry</groupId> | ||
<artifactId>opentelemetry-sdk</artifactId> | ||
<version>1.32.0</version> |
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.
Is this dependency needed?
</dependency> | ||
<dependency> | ||
<groupId>io.opentelemetry</groupId> | ||
<artifactId>opentelemetry-exporter-prometheus</artifactId> |
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 do we have a dependency to a specific exporter? Shouldn't the library work with all exporters?
* Stress test for JFR and logging UCP event listeners. | ||
* Generates realistic connection pool usage patterns to validate event recording. | ||
*/ | ||
public class JfrAndLoggingStressTestUCP { |
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 don't see any assertions, how exactly does this test work? Where do we verify that the results are correct?
* Generates realistic usage patterns to validate metric collection including | ||
* pool lifecycle, connection operations, and maintenance events. | ||
*/ | ||
public class OtelStressTestUCP { |
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 don't see any assertions, how exactly does this test work? Where do we verify that the results are correct?
* Provider that supplies a UCP event listener for recording OpenTelemetry metrics. | ||
* Converts UCP events into metrics for integration with observability platforms. | ||
*/ | ||
public final class OtelOpenTelemetryUCPEventListenerProvider implements UCPEventListenerProvider { |
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.
Do you follow the semantic conventions? https://opentelemetry.io/docs/specs/semconv/database/database-metrics/
* @return the OpenTelemetry event listener | ||
*/ | ||
@Override | ||
public UCPEventListener getListener(Map<String, String> config) { |
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.
General question to all listeners, how do you configure them? Enable/disable, sensitive data, etc?
* Configuration utility for initializing OpenTelemetry with Prometheus exporter. | ||
* Sets up metrics collection and HTTP endpoint for scraping. | ||
*/ | ||
public class OpenTelemetryConfig { |
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.
Is this at all necessary?
Implementing JFR, OTEL and Logging Providers for monitoring UCP.