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.

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

[JENKINS-71715] Allow plugins with alternate build log visualizations to handle console links from core and other plugins #8321

Merged
merged 34 commits into from
Nov 18, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
4862fa0
[JENKINS-71715] Allow plugins with alternate build log visualizations…
jgreffe Aug 1, 2023
ecc7e50
Merge branch 'master' into JENKINS-71715_first-approach
jgreffe Aug 1, 2023
d1f3aa2
Merge branch 'master' into JENKINS-71715_first-approach
jgreffe Aug 1, 2023
4316970
[JENKINS-71715] Apply suggestions with ordinal extension and @since TODO
jgreffe Aug 1, 2023
519adfd
[JENKINS-71715] Public @Extension
jgreffe Aug 2, 2023
618f538
Add or fix @since todo
batmat Aug 4, 2023
f8fa8fe
[JENKINS-71715] Apply suggestions from reviews
jgreffe Aug 4, 2023
a976eb6
[JENKINS-71715] Switch ConsoleURLProvider to interface and add Functi…
jgreffe Aug 4, 2023
d853004
[JENKINS-71715] Refactor ConsoleUrlProvider API and add Functions.get…
dwnusbaum Aug 4, 2023
2dd4e97
[JENKINS-71715] Fix casing of ConsoleURLProvider.java
dwnusbaum Aug 7, 2023
018789a
[JENKINS-71715] Add global and per-user configuration options for Con…
dwnusbaum Aug 7, 2023
3d9b41c
[JENKINS-71715] Fix Javadoc reference
dwnusbaum Aug 7, 2023
7305016
[JENKINS-71715] Remove outdated TODO comment
dwnusbaum Aug 7, 2023
6491cbb
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Aug 7, 2023
5f051c9
[JENKINS-71715] Miscellaneous updates based on review feedback
dwnusbaum Aug 8, 2023
e1b938b
[JENKINS-71715] Fix Javadoc links
dwnusbaum Aug 8, 2023
ae52b83
[JENKINS-71715] Update console links for build list and build time tr…
dwnusbaum Aug 9, 2023
46b9cda
[JENKINS-71715] Functions.getConsoleUrl must take Queue.Executable in…
dwnusbaum Aug 11, 2023
4f12b2b
[JENKINS-71715] Simplify test using WebClient.executeOnServer
dwnusbaum Aug 11, 2023
2ab2ea9
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Aug 18, 2023
c34df9d
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Aug 21, 2023
2f669da
[JENKINS-71715] Add constructor
jgreffe Aug 22, 2023
a7ac1fd
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Aug 22, 2023
f650e1f
Merge branch 'master' into JENKINS-71715_first-approach
jgreffe Sep 1, 2023
f8c0aac
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Sep 5, 2023
598ca1c
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Sep 15, 2023
848b20c
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Sep 26, 2023
48dcff8
[JENKINS-71715] Preserve old behavior of p:console-link as a fallback…
dwnusbaum Sep 26, 2023
b5420ad
[JENKINS-71715] Ignore invalid URLs and consult global providers if u…
dwnusbaum Sep 27, 2023
bcf46b6
[JENKINS-71715] Move ConsoleUrlProviderGlobalConfiguration into Appea…
dwnusbaum Sep 27, 2023
a4d5a21
[JENKINS-71715] Hide AppearanceGlobalConfiguration if ConsoleUrlProvi…
dwnusbaum Sep 28, 2023
64c3faa
[JENKINS-71715] Update help text and Javadoc based on review feedback…
dwnusbaum Oct 30, 2023
7e51da5
Merge branch 'master' into JENKINS-71715_first-approach
dwnusbaum Oct 30, 2023
88ea796
[JENKINS-71715] getProvidersDescriptors can be an instance method
dwnusbaum Oct 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions core/src/main/java/hudson/link/ConsoleURLProvider.java
dwnusbaum marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package hudson.link;
Copy link
Member

Choose a reason for hiding this comment

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

Please use jenkins.… packages for new types.

Copy link
Member

Choose a reason for hiding this comment

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

Is it better to create a jenkins.console package and put it there or put it in the already-existing hudson.console package?


import hudson.ExtensionList;
import hudson.ExtensionPoint;
import hudson.Util;
import hudson.model.Run;
import jenkins.model.Jenkins;

/**
* Extension point for providing console urls
* @since 2.417
jgreffe marked this conversation as resolved.
Show resolved Hide resolved
*/
public abstract class ConsoleURLProvider implements ExtensionPoint {

/**
* Provide the console url of the highest registered ordinal implementation.
* Defaults to {@link ConsoleURLProviderImpl} otherwise.
* @since 2.417
*/
public String getConsoleURL(Run<?, ?> run) {
return get().getConsoleURL(run);
}

/**
* Retrieve all implementations of ConsoleURLProvider.
* @since 2.417
*/
public static ExtensionList<ConsoleURLProvider> all() {
return ExtensionList.lookup(ConsoleURLProvider.class);
}

/**
* Retrieve the highest registered ordinal implementation.
* @since 2.417
batmat marked this conversation as resolved.
Show resolved Hide resolved
*/
public static ConsoleURLProvider get() {
return all().stream().findFirst().orElse(ConsoleURLProviderImpl.INSTANCE);
}

static class ConsoleURLProviderImpl extends ConsoleURLProvider {
dwnusbaum marked this conversation as resolved.
Show resolved Hide resolved

static final ConsoleURLProvider INSTANCE = new ConsoleURLProviderImpl();
dwnusbaum marked this conversation as resolved.
Show resolved Hide resolved

@Override
public String getConsoleURL(Run<?, ?> run) {
return getRoot() + Util.encode(run.getUrl()) + "console";
}

public String getRoot() {
String root = Jenkins.get().getRootUrl();
if (root == null) {
root = "http://unconfigured-jenkins-location/";
Copy link
Member

Choose a reason for hiding this comment

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

Is there a legitimate use case for when this code would be reached?

Display URL API has this because the URLs are used outside Jenkins and computed in a non-request context. It seems these URLs are always used in a request context, as they're used to render parts of the UI?

Copy link
Member

@dwnusbaum dwnusbaum Aug 3, 2023

Choose a reason for hiding this comment

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

Good point, it would probably be better to design the new API to work with URLs relative to the context root, and then use something like '/' + Stapler.getCurrentRequest().getContextPath() + getConsoleURL(run) to build the console URLs.

The implementation of the extension that should go into jenkinsci/display-url-api-plugin#194 would need to convert the absolute URLs into context path-relative URLs though, and I guess what I was thinking here would no longer apply, since we would want to use ConsoleURLProvider for all links in the Jenkins UI and DisplayUrlProvider.getConsoleUrl only when producing external links.

EDIT: Although now I wonder if there is a use case where people want to redirect to a completely separate external logging system, e.g. maybe you use opentelemetry as described here and want to direct users to some Elasticsearch frontend instead.

Copy link
Member

Choose a reason for hiding this comment

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

EDIT: Although now I wonder if there is a use case where people want to redirect to a completely separate external logging system

That seems like something that could be interesting. Summoning @cyrille-leclerc WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it some more -- just after the ping comment, sorry Cyrille -- this could also easily be implemented by having a Jenkins-internal URL that redirects to the outside system, instead of linking there directly.

Copy link
Contributor

@cyrille-leclerc cyrille-leclerc Aug 7, 2023

Choose a reason for hiding this comment

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

Thanks @daniel-beck. Regarding the OpenTelemetry integrations where Jenkins pipeline logs are persisted in an external logs management system rather than in the Jenkins home, all integrations I am aware of want to

  • Use Jenkins's built-in pipeline logs visualization UX to render the logs stored externally
  • Offer a visualization in their own logs management GUI, often workflows combining metrics, traces, and logs of the pipeline executions.

I don't see any OTel integration scenario wanting to create an alternative pipeline logs visualization GUI in Jenkins.

Does it make sense?

}
return Util.encode(root);
}
}
}
9 changes: 9 additions & 0 deletions core/src/main/java/hudson/model/Run.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import hudson.console.ConsoleNote;
import hudson.console.ModelHyperlinkNote;
import hudson.console.PlainTextConsoleOutputStream;
import hudson.link.ConsoleURLProvider;
import hudson.model.Descriptor.FormException;
import hudson.model.listeners.RunListener;
import hudson.model.listeners.SaveableListener;
Expand Down Expand Up @@ -1075,6 +1076,14 @@ protected void dropLinks() {
return getNumber() + "/";
}

/**
* Obtains the redirect console URL to this build.
* @return String like "job/foo/32/console" by default, or a String declared by an Extension of {@link ConsoleURLProvider}
dwnusbaum marked this conversation as resolved.
Show resolved Hide resolved
*/
batmat marked this conversation as resolved.
Show resolved Hide resolved
public final @NonNull String getConsoleDisplayRedirect() {
return ConsoleURLProvider.get().getConsoleURL(this);
}

/**
* Unique ID of this build.
* Usually the decimal form of {@link #number}, but may be a formatted timestamp for historical builds.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ THE SOFTWARE.
<td class="build-row-cell">
<div class="pane build-name">
<div class="build-icon">
<a class="build-status-link" href="${link}console" tooltip="${build.iconColor.description} > ${%Console Output}">
<a class="build-status-link" href="${build.consoleDisplayRedirect}" tooltip="${build.iconColor.description} > ${%Console Output}">
<l:icon class="${build.buildStatusIconClassName} icon-sm" />
</a>
</div>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/resources/lib/hudson/buildProgressBar.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ THE SOFTWARE.
<j:set var="executor" value="${executor?:build.executor}" /> <!-- TODO use Executor.of -->
<t:progressBar tooltip="${%text(executor.timestampString,executor.estimatedRemainingTime)}"
red="${executor.isLikelyStuck()}"
pos="${executor.progress}" href="${rootURL}/${build.url}console"/>
pos="${executor.progress}" href="${build.consoleDisplayRedirect}"/>
</j:jelly>
37 changes: 37 additions & 0 deletions test/src/test/java/hudson/model/RunConsoleTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package hudson.model;

import hudson.link.ConsoleURLProvider;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;

public class RunConsoleTest {

@Rule
public JenkinsRule j = new JenkinsRule();

@Test
public void defaultConsoleUrl() throws Exception {
FreeStyleProject p = j.createFreeStyleProject("something");
FreeStyleBuild b = j.buildAndAssertSuccess(p);
MatcherAssert.assertThat(b.getConsoleDisplayRedirect(), Matchers.endsWith("job/something/1/console"));
}

@Test
public void extendedConsoleUrl() throws Exception {
j.jenkins.getExtensionList(ConsoleURLProvider.class).add(0, new CustomConsoleURLProvider());
FreeStyleProject p = j.createFreeStyleProject("something");
FreeStyleBuild b = j.buildAndAssertSuccess(p);
Assert.assertEquals("job/something/1/custom", b.getConsoleDisplayRedirect());
}

public static final class CustomConsoleURLProvider extends ConsoleURLProvider {
@Override
public String getConsoleURL(Run<?, ?> run) {
return run.getUrl() + "custom";
}
}
}