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.

Sign up for GitHub

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

fix logs #537

Merged
merged 1 commit into from
Dec 23, 2024
+54 −1
Merged

fix logs #537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ spec:
tenant: {{ .Release.Namespace }}
remoteWrite:
url: http://vminsert-shortterm.{{ $targetTenant }}.svc:8480/insert/0/prometheus

fluent-bit:
readinessProbe:
httpGet:
Expand Down
54 changes: 54 additions & 0 deletions packages/system/monitoring-agents/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,3 +305,57 @@ vmagent:
tenant: tenant-root
remoteWrite:
url: http://vminsert-shortterm.tenant-root.svc:8480/insert/0/prometheus

fluent-bit:
readinessProbe:
httpGet:
path: /
daemonSetVolumes:
- name: varlog
hostPath:
path: /var/log
- name: varlibdockercontainers
hostPath:
path: /var/lib/docker/containers
daemonSetVolumeMounts:
- name: varlog
mountPath: /var/log
- name: varlibdockercontainers
mountPath: /var/lib/docker/containers
readOnly: true
config:
outputs: |
[OUTPUT]
Name http
Match kube.*
Host vlogs-generic.tenant-root.svc
port 9428
compress gzip
uri /insert/jsonline?_stream_fields=stream,kubernetes_pod_name,kubernetes_container_name,kubernetes_namespace_name&_msg_field=log&_time_field=date
format json_lines
json_date_format iso8601
header AccountID 0
header ProjectID 0
filters: |
Comment on lines +327 to +339
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance output configuration for reliability and security

The current output configuration lacks important reliability and security features:

  1. Missing retry mechanism for network failures
  2. No TLS configuration for secure communication
  3. Hard-coded AccountID/ProjectID values
     outputs: |
       [OUTPUT]
           Name http
           Match kube.*
           Host vlogs-generic.tenant-root.svc
           port 9428
           compress gzip
           uri /insert/jsonline?_stream_fields=stream,kubernetes_pod_name,kubernetes_container_name,kubernetes_namespace_name&_msg_field=log&_time_field=date
           format json_lines
           json_date_format iso8601
+          tls On
+          tls.verify On
+          retry_limit 5
+          net.keepalive On
+          net.keepalive_idle_timeout 30
           header AccountID 0
           header ProjectID 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
outputs: |
[OUTPUT]
Name http
Match kube.*
Host vlogs-generic.tenant-root.svc
port 9428
compress gzip
uri /insert/jsonline?_stream_fields=stream,kubernetes_pod_name,kubernetes_container_name,kubernetes_namespace_name&_msg_field=log&_time_field=date
format json_lines
json_date_format iso8601
header AccountID 0
header ProjectID 0
filters: |
outputs: |
[OUTPUT]
Name http
Match kube.*
Host vlogs-generic.tenant-root.svc
port 9428
compress gzip
uri /insert/jsonline?_stream_fields=stream,kubernetes_pod_name,kubernetes_container_name,kubernetes_namespace_name&_msg_field=log&_time_field=date
format json_lines
json_date_format iso8601
tls On
tls.verify On
retry_limit 5
net.keepalive On
net.keepalive_idle_timeout 30
header AccountID 0
header ProjectID 0
filters: |

[FILTER]
Name kubernetes
Match kube.*
Merge_Log On
Keep_Log On
K8S-Logging.Parser On
K8S-Logging.Exclude On
[FILTER]
Name nest
Match *
Wildcard pod_name
Operation lift
Nested_under kubernetes
Add_prefix kubernetes_
[FILTER]
Name modify
Match *
Add tenant tenant-root
[FILTER]
Name modify
Match *
Add cluster root-cluster
Comment on lines +339 to +361
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Parameterize tenant and cluster names in filters

The tenant and cluster names are currently hard-coded. These should be parameterized to support different environments and multi-tenant deployments.

     filters: |
       [FILTER]
           Name modify
           Match *
-          Add tenant tenant-root
+          Add tenant ${TENANT_NAME}
       [FILTER]
           Name modify
           Match *
-          Add cluster root-cluster
+          Add cluster ${CLUSTER_NAME}

Additionally, consider adding a filter to handle multiline logs (e.g., stack traces) before the Kubernetes filter:

     filters: |
+      [FILTER]
+          Name multiline
+          Match *
+          multiline.key_content log
+          multiline.parser java
       [FILTER]
           Name kubernetes

Committable suggestion skipped: line range outside the PR's diff.