feat: added sidecar to turn logs into metrics and forward remaining logs - #59
Open
CptSchnitz wants to merge 14 commits into
Open
feat: added sidecar to turn logs into metrics and forward remaining logs#59CptSchnitz wants to merge 14 commits into
CptSchnitz wants to merge 14 commits into
Conversation
…nt and resource limits - step 1
CptSchnitz
force-pushed
the
metrics-sidecar
branch
from
August 4, 2026 05:28
638b63b to
ce005a9
Compare
shimoncohen
requested changes
Aug 6, 2026
Comment on lines
+55
to
+56
| access_log /var/log/nginx/access.log {{ if .Values.fluentbit.enabled }}{{ if .Values.fluentbit.accessLog.stdoutReadable }}readable{{ else }}main{{ end }}; | ||
| access_log syslog:server=127.0.0.1:{{ .Values.fluentbit.accessLog.syslogPort }} main{{ else }}main{{ end }}; |
Contributor
There was a problem hiding this comment.
The nested if/else here is hard to read, suggested two options.
Option 1 — split the whole block by the outer condition. Each rendered result is visible verbatim; no directive value straddles a template boundary.
{{- if .Values.fluentbit.enabled }}
access_log /var/log/nginx/access.log {{ .Values.fluentbit.accessLog.stdoutReadable | ternary "readable" "main" }};
access_log syslog:server=127.0.0.1:{{ .Values.fluentbit.accessLog.syslogPort }} main;
{{- else }}
access_log /var/log/nginx/access.log main;
{{- end }}
Cost: the file access_log ... main; literal is duplicated across both branches — a 1-line dupe in exchange for a readable conditional.
Option 2 — name the format, keep one file directive. Zero duplication; the format decision is a named variable up top instead of inline nesting.
{{- $stdoutFormat := "main" }}
{{- if and .Values.fluentbit.enabled .Values.fluentbit.accessLog.stdoutReadable }}
{{- $stdoutFormat = "readable" }}
{{- end }}
access_log /var/log/nginx/access.log {{ $stdoutFormat }};
{{- if .Values.fluentbit.enabled }}
access_log syslog:server=127.0.0.1:{{ .Values.fluentbit.accessLog.syslogPort }} main;
{{- end }}
Both read top-to-bottom without straddling template boundaries. Option 1 is a touch clearer at the cost of one duplicated literal; Option 2 avoids the dupe with one extra variable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.