Skip to content

Fix Grafana explore URL generation and add Tempo datasource UID support#1250

Open
somiljain2006 wants to merge 11 commits into
jenkinsci:mainfrom
somiljain2006:Traceql-issue
Open

Fix Grafana explore URL generation and add Tempo datasource UID support#1250
somiljain2006 wants to merge 11 commits into
jenkinsci:mainfrom
somiljain2006:Traceql-issue

Conversation

@somiljain2006

@somiljain2006 somiljain2006 commented Mar 12, 2026

Copy link
Copy Markdown

Fixes #829

Adds optional support for a Tempo datasource UID (tempoDataSourceUid) to prevent Grafana from dropping the query when rewriting explore URLs. If the UID is not configured, the plugin falls back to the existing tempoDataSourceIdentifier to preserve backward compatibility.

Testing done

  • Added unit tests in GrafanaBackendTest to verify the generated Grafana Explore URL.
  • Verified that the URL now uses the modern schemaVersion=1&panes format instead of the legacy left parameter.
  • Confirmed that the configured tempoDataSourceUid is used in the generated URL when provided.
  • Verified backward compatibility by ensuring the plugin falls back to tempoDataSourceIdentifier when the UID is not set.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

@somiljain2006 somiljain2006 requested a review from a team as a code owner March 12, 2026 20:26
@somiljain2006

Copy link
Copy Markdown
Author

@kuisathaverat Can you please review this pr?

@kuisathaverat

Copy link
Copy Markdown
Contributor

Did you test it on Grafana? Which version?
At first look, it is a breaking change, users with previous configurations will have to change their configurations because there is a new field. The old field is not used anymore. I am not sure if this fixes anything.

@somiljain2006

somiljain2006 commented Mar 13, 2026

Copy link
Copy Markdown
Author

@kuisathaverat, thanks for taking a look at this.
Yes, I tested the change locally using Grafana 12.4.1 with a Tempo backend in a Docker setup.

I verified that when clicking “View pipeline with Grafana”, Grafana correctly opens the explore page and preserves the traceId.

Regarding the breaking change concern, this should remain backward compatible. I added explicit fallback logic if a user hasn't configured the new tempoDataSourceUid, the plugin automatically falls back to using their existing tempoDataSourceIdentifier. I also added a specific unit test (testTraceUrlFallbackToIdentifier) about this.

Without this change, Grafana rewrites the old explore URL format and drops the traceId. This update aligns the generated URL with the current Grafana explore URL structure so the traceId is preserved.

Comment thread src/main/java/io/jenkins/plugins/opentelemetry/backend/GrafanaBackend.java Outdated
@somiljain2006

Copy link
Copy Markdown
Author

@kuisathaverat, I have applied the requested changes. Can you please review it?

@ArpanC6

ArpanC6 commented Mar 19, 2026

Copy link
Copy Markdown

Great work on this fix @somiljain2006! The migration from the legacy
left parameter to the modern schemaVersion=1&panes format is the
right approach for Grafana compatibility.

A few observations after reviewing the changes:

  1. Migration logic — The readResolve() method cleanly handles
    backward compatibility by migrating tempoDataSourceIdentifier to
    tempoDataSourceUid. This is the correct Jenkins serialization pattern.

  2. Test coverage — 100% line coverage (+36.05%) is excellent. The
    5 new tests cover the key scenarios well: URL generation, fallback,
    equals/hashCode, and migration.

  3. One observation — Lines 76-77 show partial branch coverage in
    readResolve(). The missing branch appears to be when
    tempoDataSourceUid is already set to a non-default value AND
    tempoDataSourceIdentifier is also non-null. A test for this edge
    case could help close the coverage gap.

All CI checks passing including 240 tests on Linux.

@g3n35i5

g3n35i5 commented Apr 22, 2026

Copy link
Copy Markdown

Hey @kuisathaverat I hope you’re doing well! Could you please take a look at this PR when you have a moment? Getting this fix merged would be a big help for our Jenkins integration, Thank you so much! 🙏

@somiljain2006 thank you for the fix!

@g3n35i5

g3n35i5 commented May 18, 2026

Copy link
Copy Markdown

Hey @kuisathaverat! I’d like to gently ask again if you could take another look at the PR here. @somiljain2006 Is there any other way I can fix this locally in my configuration?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Grafana visualization link does not work

4 participants