From da5e287fc819edd86a1e86905381035436dfaed9 Mon Sep 17 00:00:00 2001 From: Flamki <9833ayush@gmail.com> Date: Mon, 23 Feb 2026 10:49:58 +0530 Subject: [PATCH 1/3] fix(queue): read queue metrics with SYSTEM ACL Wrap queue item collection in SYSTEM ACL for metrics callback so restricted anonymous contexts do not force jenkins.queue.count statuses to 0. Add a regression test for issue #1174 that verifies anonymous queue visibility is empty while metrics collection sees queued items. --- .../queue/MonitoringQueueListener.java | 83 +++++++++++-------- .../queue/MonitoringQueueListenerTest.java | 72 ++++++++++++++++ 2 files changed, 119 insertions(+), 36 deletions(-) create mode 100644 src/test/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListenerTest.java diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListener.java b/src/main/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListener.java index aaf517cd3..4933846d9 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListener.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListener.java @@ -9,9 +9,12 @@ import static io.jenkins.plugins.opentelemetry.semconv.JenkinsMetrics.*; import static io.jenkins.plugins.opentelemetry.semconv.JenkinsMetrics.JENKINS_QUEUE_COUNT; +import com.google.common.annotations.VisibleForTesting; import hudson.Extension; import hudson.model.Queue; import hudson.model.queue.QueueListener; +import hudson.security.ACL; +import hudson.security.ACLContext; import io.jenkins.plugins.opentelemetry.JenkinsControllerOpenTelemetry; import io.jenkins.plugins.opentelemetry.api.OpenTelemetryLifecycleListener; import io.jenkins.plugins.opentelemetry.semconv.ConfigurationKey; @@ -24,7 +27,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import java.util.Arrays; -import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; @@ -91,45 +93,41 @@ public void postConstruct() { () -> { LOGGER.log(Level.FINE, () -> "Recording Jenkins queue metrics..."); - Optional queue = - Optional.ofNullable(Jenkins.getInstanceOrNull()).map(Jenkins::getQueue); - queue.map(Queue::getItems).ifPresent(items -> { - AtomicInteger blocked = new AtomicInteger(); - AtomicInteger buildable = new AtomicInteger(); - AtomicInteger left = new AtomicInteger(); - AtomicInteger stuck = new AtomicInteger(); - AtomicInteger unknown = new AtomicInteger(); - AtomicInteger waiting = new AtomicInteger(); - Arrays.stream(items).forEach(item -> { - if (item instanceof Queue.BlockedItem) { - blocked.incrementAndGet(); - } else if (item instanceof Queue.BuildableItem) { - if (item.isStuck()) { - // buildable but here for too long - stuck.incrementAndGet(); - } else { - buildable.incrementAndGet(); - } - } else if (item instanceof Queue.WaitingItem) { - waiting.incrementAndGet(); - } else if (item instanceof Queue.LeftItem) { - left.incrementAndGet(); + Queue.Item[] items = getQueueItemsForMetrics(); + AtomicInteger blocked = new AtomicInteger(); + AtomicInteger buildable = new AtomicInteger(); + AtomicInteger stuck = new AtomicInteger(); + AtomicInteger unknown = new AtomicInteger(); + AtomicInteger waiting = new AtomicInteger(); + Arrays.stream(items).forEach(item -> { + if (item instanceof Queue.BlockedItem) { + blocked.incrementAndGet(); + } else if (item instanceof Queue.BuildableItem) { + if (item.isStuck()) { + // buildable but here for too long + stuck.incrementAndGet(); } else { - LOGGER.log(Level.INFO, () -> "Unknown item: " + item + " - class=" + item.getClass()); - unknown.incrementAndGet(); + buildable.incrementAndGet(); } - }); - queueItems.record(blocked.get(), Attributes.of(STATUS, "blocked")); - queueBlockedItems.record(blocked.get()); - queueItems.record(buildable.get(), Attributes.of(STATUS, "buildable")); - queueBuildableItems.record(buildable.get()); - queueItems.record(stuck.get(), Attributes.of(STATUS, "stuck")); - if (unknown.get() > 0) { - queueItems.record(unknown.get(), Attributes.of(STATUS, "unknown")); + } else if (item instanceof Queue.WaitingItem) { + waiting.incrementAndGet(); + } else if (item instanceof Queue.LeftItem) { + // ignore, left items are observed via onLeft() + } else { + LOGGER.log(Level.INFO, () -> "Unknown item: " + item + " - class=" + item.getClass()); + unknown.incrementAndGet(); } - queueItems.record(waiting.get(), Attributes.of(STATUS, "waiting")); - queueWaitingItems.record(waiting.get()); }); + queueItems.record(blocked.get(), Attributes.of(STATUS, "blocked")); + queueBlockedItems.record(blocked.get()); + queueItems.record(buildable.get(), Attributes.of(STATUS, "buildable")); + queueBuildableItems.record(buildable.get()); + queueItems.record(stuck.get(), Attributes.of(STATUS, "stuck")); + if (unknown.get() > 0) { + queueItems.record(unknown.get(), Attributes.of(STATUS, "unknown")); + } + queueItems.record(waiting.get(), Attributes.of(STATUS, "waiting")); + queueWaitingItems.record(waiting.get()); }, queueItems, queueWaitingItems, @@ -168,4 +166,17 @@ public void onEnterWaiting(Queue.WaitingItem wi) { } } } + + @VisibleForTesting + Queue.Item[] getQueueItemsForMetrics() { + Jenkins jenkins = Jenkins.getInstanceOrNull(); + if (jenkins == null) { + return new Queue.Item[0]; + } + // Queue#getItems is permission-filtered in recent Jenkins cores. + // Metrics are node-level health signals and must observe the full queue. + try (ACLContext ignored = ACL.as2(ACL.SYSTEM2)) { + return jenkins.getQueue().getItems(); + } + } } diff --git a/src/test/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListenerTest.java b/src/test/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListenerTest.java new file mode 100644 index 000000000..bf54d8d74 --- /dev/null +++ b/src/test/java/io/jenkins/plugins/opentelemetry/queue/MonitoringQueueListenerTest.java @@ -0,0 +1,72 @@ +/* + * Copyright The Original Author or Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.jenkins.plugins.opentelemetry.queue; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; + +import hudson.ExtensionList; +import hudson.model.FreeStyleProject; +import hudson.model.Queue; +import hudson.security.ACL; +import hudson.security.ACLContext; +import hudson.security.FullControlOnceLoggedInAuthorizationStrategy; +import jenkins.model.Jenkins; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.junit.jupiter.WithJenkins; + +@WithJenkins +class MonitoringQueueListenerTest { + + private JenkinsRule jenkinsRule; + + @BeforeEach + void beforeEach(JenkinsRule jenkinsRule) { + this.jenkinsRule = jenkinsRule; + } + + @Test + @Issue("https://github.com/jenkinsci/opentelemetry-plugin/issues/1174") + void queueMetricsReadItemsWithSystemAuthentication() throws Exception { + jenkinsRule.jenkins.setSecurityRealm(jenkinsRule.createDummySecurityRealm()); + FullControlOnceLoggedInAuthorizationStrategy authorizationStrategy = + new FullControlOnceLoggedInAuthorizationStrategy(); + authorizationStrategy.setAllowAnonymousRead(false); + jenkinsRule.jenkins.setAuthorizationStrategy(authorizationStrategy); + + int originalNumExecutors = jenkinsRule.jenkins.getNumExecutors(); + FreeStyleProject project = jenkinsRule.createFreeStyleProject("queued-project"); + try { + jenkinsRule.jenkins.setNumExecutors(0); + project.scheduleBuild2(0); + + await().atMost(15, SECONDS).until(() -> { + try (ACLContext ignored = ACL.as2(ACL.SYSTEM2)) { + return jenkinsRule.jenkins.getQueue().getItems().length > 0; + } + }); + + Queue.Item[] anonymousVisibleItems; + try (ACLContext ignored = ACL.as2(Jenkins.ANONYMOUS2)) { + anonymousVisibleItems = jenkinsRule.jenkins.getQueue().getItems(); + } + assertThat(anonymousVisibleItems.length, is(0)); + + MonitoringQueueListener listener = ExtensionList.lookupSingleton(MonitoringQueueListener.class); + Queue.Item[] metricsVisibleItems = listener.getQueueItemsForMetrics(); + assertThat(metricsVisibleItems.length, greaterThanOrEqualTo(1)); + } finally { + jenkinsRule.jenkins.getQueue().cancel(project); + jenkinsRule.jenkins.setNumExecutors(originalNumExecutors); + } + } +} From d3e50bd8d47e951a0f88e809d08cf538e360d282 Mon Sep 17 00:00:00 2001 From: Flamki <9833ayush@gmail.com> Date: Mon, 23 Feb 2026 20:45:18 +0530 Subject: [PATCH 2/3] fix: do not fail freestyle builds on missing step span purge --- .../opentelemetry/job/OtelTraceService.java | 18 ++-- .../job/OtelTraceServiceTest.java | 83 +++++++++++++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 src/test/java/io/jenkins/plugins/opentelemetry/job/OtelTraceServiceTest.java diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java b/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java index f034baa8c..acc100ddc 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java @@ -198,8 +198,7 @@ public void removePipelineStepSpanAndCloseAssociatedScopes( span.getSpanContext().getSpanId())) .findFirst() .ifPresentOrElse(FlowNodeMonitoringAction::purgeSpanAndCloseAssociatedScopes, () -> { - if (!Objects.equals( - span, Span.getInvalid())) { // recovery of a previous error, skip the invalid span + if (!isInvalidSpan(span)) { // recovery of a previous error, skip invalid spans String msg = "span not found to be purged: " + OtelUtils.toDebugString(span) + " ending " + OtelUtils.toDebugString(startSpanNode) + " in " + run; if (STRICT_MODE) { @@ -221,13 +220,22 @@ public void removeBuildStepSpan( span.getSpanContext().getSpanId())) .findFirst() .ifPresentOrElse(BuildStepMonitoringAction::purgeSpanAndCloseAssociatedScopes, () -> { - if (!Objects.equals( - span, Span.getInvalid())) { // recovery of a previous error, skip the invalid span - throw new IllegalStateException("span not found to be purged: " + span + " for " + buildStep); + if (!isInvalidSpan(span)) { // recovery of a previous error, skip invalid spans + String message = "span not found to be purged: " + OtelUtils.toDebugString(span) + " for " + + buildStep + " in " + build; + if (STRICT_MODE) { + throw new IllegalStateException(message); + } else { + LOGGER.log(Level.WARNING, message); + } } }); } + private boolean isInvalidSpan(@NonNull Span span) { + return Objects.equals(span, Span.getInvalid()) || !span.getSpanContext().isValid(); + } + public void purgeRun(@NonNull Run run) { run.getActions(OtelMonitoringAction.class).forEach(OtelMonitoringAction::purgeSpanAndCloseAssociatedScopes); // TODO verify we don't need this cleanup diff --git a/src/test/java/io/jenkins/plugins/opentelemetry/job/OtelTraceServiceTest.java b/src/test/java/io/jenkins/plugins/opentelemetry/job/OtelTraceServiceTest.java new file mode 100644 index 000000000..7e9359c13 --- /dev/null +++ b/src/test/java/io/jenkins/plugins/opentelemetry/job/OtelTraceServiceTest.java @@ -0,0 +1,83 @@ +/* + * Copyright The Original Author or Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.jenkins.plugins.opentelemetry.job; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import hudson.model.AbstractBuild; +import hudson.tasks.BuildStep; +import io.jenkins.plugins.opentelemetry.job.action.BuildStepMonitoringAction; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import java.util.Collections; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +class OtelTraceServiceTest { + + @AfterEach + void restoreStrictMode() { + OtelTraceService.STRICT_MODE = false; + } + + @Test + void removeBuildStepSpan_missingValidSpan_doesNotThrowWhenStrictModeIsDisabled() { + OtelTraceService service = new OtelTraceService(); + AbstractBuild build = mock(AbstractBuild.class); + BuildStep buildStep = mock(BuildStep.class); + when(build.getActions(BuildStepMonitoringAction.class)).thenReturn(Collections.emptyList()); + Span validSpan = Span.wrap(SpanContext.create( + "0123456789abcdef0123456789abcdef", + "0123456789abcdef", + TraceFlags.getSampled(), + TraceState.getDefault())); + + OtelTraceService.STRICT_MODE = false; + + assertDoesNotThrow(() -> service.removeBuildStepSpan(build, buildStep, validSpan)); + } + + @Test + void removeBuildStepSpan_missingValidSpan_throwsWhenStrictModeIsEnabled() { + OtelTraceService service = new OtelTraceService(); + AbstractBuild build = mock(AbstractBuild.class); + BuildStep buildStep = mock(BuildStep.class); + when(build.getActions(BuildStepMonitoringAction.class)).thenReturn(Collections.emptyList()); + Span validSpan = Span.wrap(SpanContext.create( + "0123456789abcdef0123456789abcdef", + "0123456789abcdef", + TraceFlags.getSampled(), + TraceState.getDefault())); + + OtelTraceService.STRICT_MODE = true; + + assertThrows(IllegalStateException.class, () -> service.removeBuildStepSpan(build, buildStep, validSpan)); + } + + @Test + void removeBuildStepSpan_missingInvalidSpan_doesNotThrowEvenWhenStrictModeIsEnabled() { + OtelTraceService service = new OtelTraceService(); + AbstractBuild build = mock(AbstractBuild.class); + BuildStep buildStep = mock(BuildStep.class); + when(build.getActions(BuildStepMonitoringAction.class)).thenReturn(Collections.emptyList()); + Span invalidSpan = Span.wrap(SpanContext.create( + "00000000000000000000000000000000", + "0000000000000000", + TraceFlags.getDefault(), + TraceState.getDefault())); + + assertFalse(invalidSpan.getSpanContext().isValid()); + OtelTraceService.STRICT_MODE = true; + + assertDoesNotThrow(() -> service.removeBuildStepSpan(build, buildStep, invalidSpan)); + } +} From f1bc1dcd2c7800046833f761633c99cd18fba49f Mon Sep 17 00:00:00 2001 From: Flamki <9833ayush@gmail.com> Date: Thu, 19 Mar 2026 11:03:39 +0530 Subject: [PATCH 3/3] fix: add fine log when span purge is missing in non-strict mode --- .../io/jenkins/plugins/opentelemetry/job/OtelTraceService.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java b/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java index acc100ddc..14bffeff9 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/job/OtelTraceService.java @@ -204,6 +204,7 @@ public void removePipelineStepSpanAndCloseAssociatedScopes( if (STRICT_MODE) { throw new IllegalStateException(msg); } else { + LOGGER.log(Level.FINE, msg); LOGGER.log(Level.WARNING, msg); } } @@ -226,6 +227,7 @@ public void removeBuildStepSpan( if (STRICT_MODE) { throw new IllegalStateException(message); } else { + LOGGER.log(Level.FINE, message); LOGGER.log(Level.WARNING, message); } }