Make OTEL endpoint types self-describing - #505
Draft
matt-richardson wants to merge 6 commits into
Draft
Conversation
Each endpoint handler now declares its own service name and front-end assets, and its resources live in a per-service directory. Adding a backend no longer means editing the factory switch, the settings JSP's hardcoded option list and includes, or the JavaScript show/hide chain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BuildServerAdapter's buildFinished and buildInterrupted are empty, so the ordering carries no behaviour; this keeps the two overrides consistent with each other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous pattern only matched build directories one level deep, so the root build/ produced by the distribution task showed as untracked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Handlers now declare their own service name and asset paths as strings, so a typo or a handler left out of the factory is only visible when the settings page is rendered in TeamCity. These assert that every service resolves to a handler claiming it, and that every declared resource is on the classpath. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The service list and the per-service fragment paths drive the form whether or not the project has the plugin feature yet, so they belong in the model unconditionally. The JSP declares both with jsp:useBean, which fails the whole page when either is absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
show() has to reverse everything hide() does, or switching away from Honeycomb and back leaves the mode row missing. The environment row it also referenced is not part of the form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
DPT-3210 (Adding an OpenTelemetry backend means editing shared files in the TeamCity plugin)
Each OpenTelemetry backend was only half self-contained. The factory resolving them, the settings JSP and the settings JavaScript each carried a hardcoded list of every service, and all the fragments and scripts shared one flat resources directory. Adding Jaeger or Sumo meant touching several files unrelated to the new backend, and missing one produced a settings page that rendered the wrong fields rather than a build error.
Results
A handler now declares its own service name and its own JS, CSS and JSP fragment paths, and its resources live in a per-service directory. The dropdown, the fragment includes and the show/hide behaviour are all driven off that, so a new backend is a handler plus a directory.
Notes
Some things worth knowing before reviewing:
The Honeycomb Mode dropdown (Classic / Environments) renders but is not wired up on the server — nothing parses
honeycombModeoff the form, nothing maps it back into the model, and there is no constant for it. Selecting a mode has no persistent effect. It is left as-is here rather than removed, sinceHoneycombOTELEndpointHandlerstill carries the//todo: add a setting to say "use classic" or "use environments"that it belongs to. It needs a decision: finish the persistence or drop the row.Two of the commits are fixes for defects introduced earlier in this branch, kept separate so they are reviewable on their own. Both only showed up when driving the real settings page in a browser: the tab failed to render at all for a project that had never been configured (the JSP declares the service list with
jsp:useBean, which fails the page when the attribute is absent, and it was only being set once a project feature existed), and switching away from Honeycomb and back left the Mode row hidden becauseshow()did not reverse everythinghide()did.There is now a unit test asserting every declared resource path resolves on the classpath. That is deliberately broader than it looks — paths are now strings spread across handlers, so a typo is easy and otherwise invisible until the page renders.
Follow-up
Browser-driven integration tests covering the settings page exist but are not included here — they add a Testcontainers and Playwright module and warrant their own review. Without them the two defects above have no automated coverage, since the unit suite cannot reach either path.
🤖 Generated with Claude Code