Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Initial Observability extension - devservices, devresources, LGTM #38448

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Initial Observability extension - devservices, devresources, LGTM #38448

merged 1 commit into from
Apr 19, 2024

Conversation

alesj
Copy link
Contributor

@alesj alesj commented Jan 29, 2024

This is a simpler version of the previous (see below) PR.
This one just includes LGTM (https://github.com/grafana/docker-otel-lgtm) support with the new devresource mechanism.
The stuff mentioned below will be added in the next PR(s).

Fixes #26445

This adds dynamic lookup of possible observability dev resources / services.
It also adds REST clients for PromQL (Prometheus query language) and VictoriaMetrics REST api support; push, etc

The dev resources / services can be run in 3 different ways

  1. re-usable dev services (default)
  2. simple start/stop dev services
  3. single / plain per QuarkusTestResource dev resource

(1) can be configured
(2) depends what's available on the classpath
(3) needs to be explicitly used in QuarkusTestResource

Currently supported dev services

  • Grafana
  • Jaeger
  • OTel
  • VictoriaMetrics DB
  • VictoriaMetrics Agent / scraper

It should be easy to add new / other observability dev services in the future,
e.g. currently missing Prometheus, etc
by adding a new DevResourceLifecycleManager implementation.

Current TODO list

  • test with 2 app running at the same time, propagating tracing to same dev service(s)
  • Grafana-all-in-one Docker image support
  • Prometheus dev service
  • reload and other (combo) tests

@quarkus-bot quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation labels Jan 29, 2024
@alesj
Copy link
Contributor Author

alesj commented Jan 29, 2024

Just remembered I can add back the devresource vs devservice (autodiscovery) tests -- but need to adjust them to LGTM usage.

@alesj
Copy link
Contributor Author

alesj commented Jan 29, 2024

Documentation is WIP -- first lets discuss if the concept is suitable enough to handle the things we want with observability and then hopefully beyond.
e.g. the issue @cescoffier just mention previous week, with multi-devservices

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 29, 2024 •
edited
Loading

🙈 The PR is closed and the preview is expired.

cescoffier
cescoffier reviewed Jan 29, 2024
View reviewed changes
Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.idea files should be removed from git.

cescoffier
cescoffier requested changes Jan 29, 2024
View reviewed changes
extensions/devservices/common/src/main/java/io/quarkus/devservices/common/ContainerLocator.java
/**
* @return container id, if exists
*/
public Optional<String> locateContainer(String serviceName, boolean shared, LaunchMode launchMode,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about "tests"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests for an existing container usage?
Do we have some already?

I guess I need to force the redeploy / restart of the test somehow,
and see if the container is still up-n-running?

...ity-devservices/common/src/main/java/io/quarkus/observability/common/ContainerConstants.java Outdated

// Images

public static final String LGTM = "grafana/otel-lgtm:0.2.0";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefix it with docker.io

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

...ity-devservices/common/src/main/java/io/quarkus/observability/common/config/ConfigUtils.java Outdated
public class ConfigUtils {

public static boolean isEnabled(ContainerConfig config) {
if (config != null && config.enabled()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to check the Quarkus runtime classloader or build classloader?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually forgot to delete this class, as it's not used anymore.

...ces/deployment/src/main/java/io/quarkus/observability/deployment/ObservabilityProcessor.java Outdated

@BuildSteps(onlyIfNot = IsNormal.class, onlyIf = { GlobalDevServicesConfig.Enabled.class,
ObservabilityProcessor.IsEnabled.class })
class ObservabilityProcessor {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you should add DevService in the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed.

...ces/deployment/src/main/java/io/quarkus/observability/deployment/ObservabilityProcessor.java Outdated

@BuildStep
FeatureBuildItem feature() {
return new FeatureBuildItem(Feature.OBSERVABILITY);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it doesn't hurt, if you want to reference it in some other extension?
Or what's the use case then?
(I saw that all of the extension processors had this ...)

@alesj
Copy link
Contributor Author

alesj commented Jan 29, 2024

.idea files should be removed from git.

Yeah, no idea how they ended up in in the first place.
Or how they were deleted ...

@alesj alesj force-pushed the obs_lgtm branch 3 times, most recently from 002fbba to 6f43cf8 Compare January 29, 2024 16:11
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat
Copy link
Contributor

brunobat commented Jan 30, 2024 •
edited
Loading

@alesj Can you please add a guide with documentation on how to use the extension?
A new telemetry-observability-devservices.adoc filename could work. The telemetry prefix is a convention for OTel, Micrometer related guides that we started to use recently.

@alesj
Copy link
Contributor Author

alesj commented Jan 30, 2024

@alesj Can you please add a guide with documentation on how to use the extension? A new telemetry-observability-devservices.adoc filename could work. The telemetry prefix is a convention for OTel, Micrometer related guides that we started to use recently.

Well, it won't be just telemetry, as this guide will be a generic one, with explicit examples for all sorts of tech - that we add later on (from the old PR).
So I think we need a better name - what about just observability-devservices?

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@alesj alesj force-pushed the obs_lgtm branch 2 times, most recently from ee83dfc to 4dbf540 Compare April 9, 2024 08:31
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@alesj
Initial Observability extension - devservices, devresources, LGTM
796348d 
Grafana OTel LGTM additional Documentation
Fix enforcer configuration and make names consistent
Update descriptors with latest dependency state
Abstract Testcontainers usage, own Container SPI.
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 19, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 796348d.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot
Copy link

quarkus-bot bot commented Apr 19, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 796348d.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
✖ MicroProfile TCKs Tests Verify Failures Logs Raw logs 🔍

Full information is available in the Build summary check run.
You can consult the Develocity build scans.

Failures

⚙️ MicroProfile TCKs Tests #

- Failing: tcks/microprofile-opentelemetry

📦 tcks/microprofile-opentelemetry

✖ org.eclipse.microprofile.telemetry.tracing.tck.async.JaxRsClientAsyncTest.testIntegrationWithJaxRsClientAsync - History - More details - Source on GitHub

java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:934)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:351)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:344)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:836)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:827)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:817)
	at io.vertx.core.impl.EventLoopExecutor.execute(EventLoopExecutor.java:35)

@brunobat brunobat merged commit b98e608 into quarkusio:main Apr 19, 2024
54 of 55 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone Apr 19, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Apr 19, 2024
@brunobat
Copy link
Contributor

Thanks very much @alesj for the effort!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartwdouglas stuartwdouglas stuartwdouglas left review comments

@fstab fstab fstab left review comments

@ozangunalp ozangunalp ozangunalp approved these changes

@cescoffier cescoffier cescoffier approved these changes

@brunobat brunobat brunobat approved these changes

@gsmet gsmet Awaiting requested review from gsmet

Assignees
No one assigned
Labels
area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation kind/enhancement New feature or request release/noteworthy-feature triage/flaky-test
Projects
Quarkus Documentation
Status: Done
Milestone
3.11.0.CR1
Development

Successfully merging this pull request may close these issues.

OpenTelemetry Collector for DevServices
7 participants
@alesj @brunobat @ozangunalp @codespearhead @stuartwdouglas @fstab @cescoffier