Skip to content

Add port 8889#748

Open
agueijo wants to merge 1 commit intojaegertracing:mainfrom
agueijo:patch-1
Open

Add port 8889#748
agueijo wants to merge 1 commit intojaegertracing:mainfrom
agueijo:patch-1

Conversation

@agueijo
Copy link
Copy Markdown

@agueijo agueijo commented Mar 10, 2026

Add port 8889 on deployment to export span-metrics

What this PR does

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format,
will close that issue when PR gets merged)

  • fixes #

Checklist

  • DCO signed
  • Commits are GPG signed
  • Chart Version bumped
  • Title of the PR starts with chart name ([jaeger] or [jaeger-operator])
  • README.md has been updated to match version/contain new values

Add port 8889 on deployment to export span-metrics

Signed-off-by: agueijo <agueijo@gmail.com>
Copilot AI review requested due to automatic review settings March 10, 2026 15:09
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds port 8889 to the Jaeger deployment container spec to expose span-metrics from the collector.

Changes:

  • Added container port 8889 (TCP) to the Jaeger deployment template alongside the existing port 8888.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

protocol: TCP
- containerPort: 8888
protocol: TCP
- containerPort: 8889
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The existing port 8888 and other ports in this file likely have a name field to identify their purpose (e.g., for use in Service selectors or readability). This new port entry is missing a name field. Consider adding a descriptive name such as span-metrics to be consistent with the other port definitions and to make it clear what this port is used for.

Suggested change
- containerPort: 8889
- containerPort: 8889
name: span-metrics

Copilot uses AI. Check for mistakes.
@Stevenpc3
Copy link
Copy Markdown
Contributor

this should not be required. This PR already exposed the necessary service port 240e4a0

We are currently using prometheus to store span metrics since this PR went in.

Also the containerPort does nothing.

{538CB0B1-7517-45C5-88F7-01287120519B}

@Stevenpc3
Copy link
Copy Markdown
Contributor

This can be merged though with no harm and is still good information to add.

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.

3 participants