Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(catalog): capabilities properties #1185

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Mar 19, 2024

With this PR we enrich the catalog and let certain runtime aspects to be managed via properties (with something similar to this draft [1]). Together with a work on the operator, we'll be able to simplify the management of runtimes and offload the specific runtime aspects, being able to delegate this to the runtime directly. The result of this PR will enrich 3.8.0 future catalog as following:

      health:
        dependencies:
        - groupId: org.apache.camel.quarkus
          artifactId: camel-quarkus-microprofile-health
        metadata:
        - key: defaultLivenessProbePath
          value: /q/health/live
        - key: defaultReadinessProbePath
          value: /q/health/ready
        - key: defaultStartupProbePath
          value: /q/health/started
...
      logging:
        runtimeProperties:
        - key: color
          value: quarkus.console.color
        - key: format
          value: quarkus.log.console.format
        - key: json
          value: quarkus.log.console.json
        - key: jsonPrettyPrint
          value: quarkus.log.console.json.pretty-print
        - key: level
          value: quarkus.log.level
...
      master:
        dependencies:
        - groupId: org.apache.camel.k
          artifactId: camel-k-master
        runtimeProperties:
        - key: labelKeyFormat
          value: quarkus.camel.cluster.kubernetes.labels."%s"
        - key: resourceName
          value: quarkus.camel.cluster.kubernetes.resource-name
        - key: resourceType
          value: quarkus.camel.cluster.kubernetes.resource-type
        buildTimeProperties:
        - key: enabled
          value: quarkus.camel.cluster.kubernetes.enabled
...
      telemetry:
        dependencies:
        - groupId: org.apache.camel.quarkus
          artifactId: camel-quarkus-opentelemetry
        runtimeProperties:
        - key: endpoint
          value: quarkus.opentelemetry.tracer.exporter.otlp.endpoint
        - key: sampler
          value: quarkus.opentelemetry.tracer.sampler
        - key: samplerParentBased
          value: quarkus.opentelemetry.tracer.sampler.parent-based
        - key: samplerRatio
          value: quarkus.opentelemetry.tracer.sampler.ratio
        - key: serviceName
          value: quarkus.opentelemetry.tracer.resource-attributes

Eventually any other runtime (ie Springboot) should be adding the specific capabilities accordingly and it would be transparent for the operator.

[1] apache/camel-k#5251

Release Note

feat(catalog): capabilities properties

@squakez squakez marked this pull request as draft March 19, 2024 09:52
@squakez squakez force-pushed the feat/trait_properties branch from c12087a to d9f5df5 Compare March 19, 2024 09:59
properties.add(Property.from("serviceName", "quarkus.opentelemetry.tracer.resource-attributes"));
properties.add(Property.from("sampler", "quarkus.opentelemetry.tracer.sampler"));
properties.add(Property.from("samplerRatio", "quarkus.opentelemetry.tracer.sampler.ratio"));
properties.add(Property.from("samplerParentBased", "quarkus.opentelemetry.tracer.sampler.parent-based"));
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be nice to validate if i.e. the same keys also apply to spring-boot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. I think this would be the next step when we onboard the new runtime. We'd need to understand how to harmonize the parameters expected by the operator according to the different runtimes requirements. The goal of this PR is to move towards that goal without breaking compatibility of what's expected now. Eventually it will be a matter of changing these parameters in the future catalog releases, driven by the changes in the operator as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there are many more questions that must be answered, i.e. :

  • how to handle cases where a key is not applicable, but another one is needed
  • how to handle breaking changes, so properties that have disappeared or become required ('d like to avoid having to do version checks)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I acknowledge them but this PR doesn't aim to solve this aspect. This is something we need to address regardless this PR. Do you think that challenge can be tackled separately?

properties.add(Property.from("color", "quarkus.console.color"));
properties.add(Property.from("format", "quarkus.log.console.format"));
properties.add(Property.from("json", "quarkus.log.console.json"));
properties.add(Property.from("jsonPrettyPrint", "quarkus.log.console.json.pretty-print"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a json logging option in spring-boot https://docs.spring.io/spring-boot/docs/3.0.0/reference/htmlsingle/#features.logging, it seems that to enable json logging one has to use a backend specific logging configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. Same comment as with the above telemetry capability.

@squakez squakez force-pushed the feat/trait_properties branch from d9f5df5 to 238d514 Compare March 19, 2024 12:32
@squakez
Copy link
Contributor Author

squakez commented Mar 19, 2024

@lburgazzoli I've addressed the metadata properties. As mentioned for the other concern, although it's valid I think it is not relevant to this PR and will have to be solved regardless of this. Let me know if you have any objection to merge, so I can start the related work on the operator side.

@squakez squakez marked this pull request as ready for review March 19, 2024 14:25
@claudio4j
Copy link
Contributor

Is it planned to backport to 3.2.x / 3.6.x for backward compatibility ?

@claudio4j
Copy link
Contributor

This idea in general is good as the description says, so +1.

@squakez
Copy link
Contributor Author

squakez commented Mar 19, 2024

Is it planned to backport to 3.2.x / 3.6.x for backward compatibility ?

No. The compatibility will be managed at runtime, ie, 3.2 runtime mechanism will work in its own way, according to its spec. The operator will still have to handle (and deprecated as illustrated in https://github.com/apache/camel-k/pull/5251/files#diff-22dc2842d3dc75fd50c5856a16c240e8c17dc3f9785d8ab14dc6eb6a990134beR70) also the older approach which will eventually disappear after deprecation policy has expired.

@squakez squakez merged commit aeffbc2 into apache:main Mar 20, 2024
22 checks passed
@squakez squakez deleted the feat/trait_properties branch March 20, 2024 09:31
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