Skip to content

feat: Config logging via helm #6312

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

Merged
merged 13 commits into from
Mar 17, 2025
Merged

Conversation

sakoush
Copy link
Contributor

@sakoush sakoush commented Mar 13, 2025

What this PR does / why we need it:

This PR introduces logging level configuration, exposed via helm.

Different components can set individual log levels or use a global setting in helm.

For the global setting, users should set logging.logLevel, possible values are: debug, info, error.

If set on individual components, then the allowed levels depend on the underlying logging used for the component. Check docs for more information.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

  • Add docs

@sakoush sakoush added the v2 label Mar 13, 2025
@sakoush sakoush marked this pull request as ready for review March 14, 2025 11:19
@sakoush sakoush requested a review from lc525 as a code owner March 14, 2025 11:19
Sherif Akoush added 2 commits March 14, 2025 11:23
@@ -53,6 +53,10 @@ spec:
value: '{{ .Values.security.kafka.ssl.client.endpointIdentificationAlgorithm }}'
- name: SELDON_CORES_COUNT
value: '{{ .Values.dataflow.cores }}'
- name: SELDON_LOG_LEVEL_APP
value: '{{ hasKey .Values.dataflow "logLevel" | ternary .Values.dataflow.logLevel .Values.logging.logLevel | upper }}'
- name: SELDON_LOG_LEVEL_KAFKA
Copy link
Member

Choose a reason for hiding this comment

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

Was initially wondering whether we won't need a similar setup to the go one on the dataflow-engine side, but it appears that it knows how to take the same levels as syslog.

Copy link
Member

@lc525 lc525 left a comment

Choose a reason for hiding this comment

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

lgtm

@sakoush sakoush merged commit bf74432 into SeldonIO:v2 Mar 17, 2025
6 checks passed
jtayl222 pushed a commit to jtayl222/seldon-core that referenced this pull request Jul 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants