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

Otel statefulset cloudwatch collector #2982

Merged
merged 7 commits into from
Apr 20, 2023
Merged

Conversation

rnishtala-sumo
Copy link
Contributor

@rnishtala-sumo rnishtala-sumo commented Apr 13, 2023

This PR creates a statefulset for collecting logs from cloudwatch. It also documents some of the steps needed to setup log ingestion from cloudwatch. The fargate readme has been updated with some details

https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/otel-statefulset-collector/docs/fargate.md#logs

Checklist

  • Changelog updated or skip changelog label added
  • Documentation updated
  • Template tests added for new features
  • Integration tests added or modified for major features

@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch from a361d8a to 3cf0db0 Compare April 13, 2023 02:13
@github-actions github-actions bot added the documentation documentation label Apr 13, 2023
@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch 4 times, most recently from 7545d01 to 8fcc775 Compare April 13, 2023 03:27
@rnishtala-sumo rnishtala-sumo changed the title Otel statefulset collector Otel statefulset cloudwatch collector Apr 13, 2023
deploy/helm/sumologic/conf/logs/otelcol/config.yaml Outdated Show resolved Hide resolved
docs/fargate.md Outdated Show resolved Hide resolved
@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch 9 times, most recently from fe92282 to 142c27b Compare April 14, 2023 02:46
@rnishtala-sumo rnishtala-sumo marked this pull request as ready for review April 14, 2023 03:00
@rnishtala-sumo rnishtala-sumo requested a review from a team as a code owner April 14, 2023 03:00
@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch 2 times, most recently from cf3f1af to b8b8e42 Compare April 14, 2023 03:33
docs/fargate.md Outdated Show resolved Hide resolved
docs/fargate.md Show resolved Hide resolved
docs/fargate.md Outdated Show resolved Hide resolved
docs/fargate.md Show resolved Hide resolved
docs/fargate.md Outdated Show resolved Hide resolved
docs/fargate.md Outdated Show resolved Hide resolved
@aboguszewski-sumo
Copy link
Contributor

Some additional thoughts after using this:

  • we should probably decide which log groups we're going to observe: with our suggested fluent-bit config there's going to be only one group, can it be more groups with different configs?
  • current way of providing awscloudwatch receiver config is a bit tiresome, for example I tried to use the following config to read log from a specific group:
sumologic:
  logs:
    collector:
      otelcloudwatch:
        awscloudwatch:
          logs:
            groups:
              named:
                fluent-bit-cloudwatch:
                  names: ["some-name"]

and it raised an error, because the default is also using autodiscover, and both autodiscover and named can't be used at the same time. So we should probably either have this receiver's config empty as default or don't copy it directly as it was suggested in one of the comments.

  • persistency for the collector definitely should be configurable (disabling it manually is a mess) and it should be explained in more detail how to enable it on Fargate
  • in general, we should have some common section about how to enable persistency on Fargate that will be common for the collector, metrics and logs

docs/fargate.md Outdated Show resolved Hide resolved
docs/fargate.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

Overally LGTM, some minor minor comments

docs/fargate.md Outdated Show resolved Hide resolved
docs/fargate.md Outdated Show resolved Hide resolved
docs/fargate.md Outdated Show resolved Hide resolved
@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch from 45f6f66 to eed8faa Compare April 19, 2023 20:20
@rnishtala-sumo
Copy link
Contributor Author

rnishtala-sumo commented Apr 20, 2023

I've made the suggested changes, will keep this documentation task below open:

https://jira.kumoroku.com/jira/browse/SUMO-205563

Added the comment from this PR to the above ticket: #2982 (comment)

Comment on lines 4392 to 4434
statefulset:
nodeSelector: {}
tolerations: []
topologySpreadConstraints: []
affinity: {}
## Acceptable values for podAntiAffinity:
## soft: specifies preferences that the scheduler will try to enforce but will not guarantee (Default)
## hard: specifies rules that must be met for a pod to be scheduled onto a node
podAntiAffinity: "soft"
replicaCount: 1
resources:
limits:
memory: 1Gi
cpu: 1000m
requests:
memory: 768Mi
## Warning! Increasing the CPU requests for Fluentd above 1000m might result in broken autoscaling
## ref: https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/main/docs/best-practices.md#cpu-resources-warning
cpu: 500m
## Option to define priorityClassName to assign a priority class to pods.
priorityClassName:

## Add custom labels only to logs otel sts pods
podLabels: {}
## Add custom annotations only to logs otel sts pods
podAnnotations: {}
## Set securityContext for containers running in pods in otelcol-instrumentation statefulset.
containers:
otelcol:
securityContext: {}
livenessProbe:
initialDelaySeconds: 15
periodSeconds: 15
timeoutSeconds: 10
failureThreshold: 3
readinessProbe:
initialDelaySeconds: 5
periodSeconds: 10
timeoutSeconds: 3
failureThreshold: 3
startupProbe:
periodSeconds: 3
failureThreshold: 60
Copy link
Contributor

Choose a reason for hiding this comment

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

Please doucment it in README as github actions are failing, also, I think it's not otellogs anymore, eventually otellogs.cloudwatch or otelcloudwatch

@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch 6 times, most recently from 247d59e to 3e07acc Compare April 20, 2023 15:45
Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

LGTM

@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch from 3e07acc to a730e0f Compare April 20, 2023 16:12
@rnishtala-sumo rnishtala-sumo force-pushed the otel-statefulset-collector branch from a730e0f to 6e146de Compare April 20, 2023 16:14
@rnishtala-sumo rnishtala-sumo merged commit 4e33e05 into main Apr 20, 2023
@rnishtala-sumo rnishtala-sumo deleted the otel-statefulset-collector branch April 20, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants