You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/05/29 17:02:29 UTC
[GitHub] [camel-k] astefanutti opened a new pull request #1478: Make Prometheus trait compatible with Knative Serving
astefanutti opened a new pull request #1478:
URL: https://github.com/apache/camel-k/pull/1478
Fixes #1459.
**Release Note**
```release-note
fix: Prometheus scraping now works with Knative Serving
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] nicolaferraro commented on a change in pull request #1478: Make Prometheus trait compatible with Knative Serving
Posted by GitBox <gi...@apache.org>.
nicolaferraro commented on a change in pull request #1478:
URL: https://github.com/apache/camel-k/pull/1478#discussion_r433171169
##########
File path: pkg/trait/prometheus.go
##########
@@ -121,10 +121,13 @@ func (t *prometheusTrait) Apply(e *Environment) (err error) {
options := []string{strconv.Itoa(t.Port), path.Join(prometheusJmxExporterConfigMountPath, prometheusJmxExporterConfigFileName)}
container.Args = append(container.Args, "-javaagent:dependencies/io.prometheus.jmx.jmx_prometheus_javaagent-0.3.1.jar="+strings.Join(options, ":"))
- // Add the container port
- containerPort := t.getContainerPort()
- container.Ports = append(container.Ports, *containerPort)
- condition.Message = fmt.Sprintf("%s(%s/%d)", container.Name, containerPort.Name, containerPort.ContainerPort)
+ // Skip declaring the Prometheus port when Knative is enabled,
+ // as only one container port is supported.
+ if e.DetermineProfile() != v1.TraitProfileKnative {
Review comment:
Not sure about this. Knative profile means that knative is present in the cluster, while the integration is not automatically exposed as Knative service (e.g. think to timer to log). Should we check the actual controller type?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #1478: Make Prometheus trait compatible with Knative Serving
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1478:
URL: https://github.com/apache/camel-k/pull/1478#discussion_r433737856
##########
File path: pkg/trait/prometheus.go
##########
@@ -121,10 +121,13 @@ func (t *prometheusTrait) Apply(e *Environment) (err error) {
options := []string{strconv.Itoa(t.Port), path.Join(prometheusJmxExporterConfigMountPath, prometheusJmxExporterConfigFileName)}
container.Args = append(container.Args, "-javaagent:dependencies/io.prometheus.jmx.jmx_prometheus_javaagent-0.3.1.jar="+strings.Join(options, ":"))
- // Add the container port
- containerPort := t.getContainerPort()
- container.Ports = append(container.Ports, *containerPort)
- condition.Message = fmt.Sprintf("%s(%s/%d)", container.Name, containerPort.Name, containerPort.ContainerPort)
+ // Skip declaring the Prometheus port when Knative is enabled,
+ // as only one container port is supported.
+ if e.DetermineProfile() != v1.TraitProfileKnative {
Review comment:
I've updated the logic to rely on the controller strategy used for the integration rather than the trait profile. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #1478: Make Prometheus trait compatible with Knative Serving
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1478:
URL: https://github.com/apache/camel-k/pull/1478#discussion_r433699647
##########
File path: pkg/trait/prometheus.go
##########
@@ -121,10 +121,13 @@ func (t *prometheusTrait) Apply(e *Environment) (err error) {
options := []string{strconv.Itoa(t.Port), path.Join(prometheusJmxExporterConfigMountPath, prometheusJmxExporterConfigFileName)}
container.Args = append(container.Args, "-javaagent:dependencies/io.prometheus.jmx.jmx_prometheus_javaagent-0.3.1.jar="+strings.Join(options, ":"))
- // Add the container port
- containerPort := t.getContainerPort()
- container.Ports = append(container.Ports, *containerPort)
- condition.Message = fmt.Sprintf("%s(%s/%d)", container.Name, containerPort.Name, containerPort.ContainerPort)
+ // Skip declaring the Prometheus port when Knative is enabled,
+ // as only one container port is supported.
+ if e.DetermineProfile() != v1.TraitProfileKnative {
Review comment:
Ah you're right, let me rework it!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti merged pull request #1478: Make Prometheus trait compatible with Knative Serving
Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #1478:
URL: https://github.com/apache/camel-k/pull/1478
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org