[jaeger] Add configurable OLTP port names in jaeger collector services

cnvergence commented 1 year ago

What this PR does

Adding a prefix to OLTP service port names, resolves issues with istio. As per https://github.com/jaegertracing/helm-charts/issues/344#issuecomment-1583708032 it can unblock traces for oltp services.


Info [IST0118] (Service jaeger-collector.test) Port name oltp-http (port: 4318, targetPort: 0) doesn't follow the naming convention of Istio port.
Info [IST0118] (Service jaeger-collector.test) Port name oltp-grpc (port: 4317, targetPort: 0) doesn't follow the naming convention of Istio port.

Potentially possible problems similar to this old issue.

Which issue this PR fixes

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

https://github.com/jaegertracing/helm-charts/issues/344#issuecomment-1100166786 (partially)

Checklist

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

Hronom commented 1 year ago

Thank you! Please merge this

mehta-ankit commented 1 year ago

My 2 cents: We should keep the chart flexible and it should not be in the business of supporting 1 or the other 3rd party tech like Istio in this example. Unless of course its a fundamental thing like K8s API we should support etc.

@naseemkullah @pavelnikolov thoughts ?

mehta-ankit commented 1 year ago

looks like something is wrong with the values file https://github.com/jaegertracing/helm-charts/actions/runs/5327037233/jobs/9650182903?pr=478

mehta-ankit commented 1 year ago

Its still not fixed

cnvergence commented 1 year ago

Should be fine now, previous mistake jumped back in.

jaegertracing / helm-charts