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

Clarification on removal of logging templates #88

Closed
romogo17 opened this issue Feb 13, 2023 · 7 comments
Closed

Clarification on removal of logging templates #88

romogo17 opened this issue Feb 13, 2023 · 7 comments

Comments

@romogo17
Copy link
Contributor

Hi team,

I was going over some documentation associated to this repo today:

However, I noticed that in #82, the templates for logging were removed from the helm chart, citing "stability of Logs upstream in the OTel community in 2023". I don't see any further issues or rationale behind this in the PR.

After installing the Helm chart I don't see any container logs in CloudWatch.

@romogo17
Copy link
Contributor Author

Just found this warning in the README

Warning: Fluent Bit and Fargate logging templates are deprecated and will be removed from the Helm chart on December 30th, 2022

Given the content, why was this feature removed before the upstream feature is stabilized and available?

@mhausenblas
Copy link
Member

@romogo17 ADOT doesn't support logs in GA yet, this was simply a cleanup to address that fact. Happy to jump on a call to discuss this is and learn more about your requirements.

@romogo17
Copy link
Contributor Author

Thanks for the response @mhausenblas. I guess I'm just a bit confused as to why that functionality was cleaned up if the replacement with ADOT doesn't support logs in GA yet.

Sure, let's have a quick call. I'll send you an invite from my company's email address.

@mbeaucha
Copy link

Can you guys post an update of the rationale you discussed here? We too were relying on this Helm Chart to provide the fluent-bit installation as @romogo17 mentioned. Is there another chart available elsewhere for fluent-bit? The AWS Documentation still refers to this project (https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Container-Insights-EKS-otel.html) as providing it as well.

@mhausenblas
Copy link
Member

@mbeaucha have a look at https://github.com/aws/eks-charts/tree/master/stable/aws-for-fluent-bit and yes, we need to update the docs everywhere as well, to clarify this.

@romogo17
Copy link
Contributor Author

I ended up giving the aws-for-fluent-bit Helm Chart a try. Seems to be working — it's a shame it's not better documented.

CAVEATS

  1. As of the day of writing, the latest chart version is 0.1.22, which uses app version 2.21.5. This version is very old and has a lot of vulnerabilities. I don't know why the Helm chart hasn't been updated in so long. I used 2.31.2 and it seems work fine.
  2. Your worker nodes or K8s service account (through IRSA) will need permissions to CloudWatch. If you're using this AWS-managed policy CloudWatchAgentServerPolicy, and want to setup logRetentionDays, this policy is not enough
i-000011112222 is not authorized to perform: logs:PutRetentionPolicy on resource: arn:aws:logs:us-east-1:222333444:log-group:/aws/eks/fluentbit-cloudwatch/logs:log-stream: because no identity-based policy allows the logs:PutRetentionPolicy action
  1. Typically containerinsights creates 3 log groups

    • /aws/containerinsights/cluster-name/dataplane
    • /aws/containerinsights/cluster-name/dataplane
    • /aws/containerinsights/cluster-name/dataplane

    This other helm chart only creates one: /aws/eks/fluentbit-cloudwatch/logs. This is configurable using the cloudWatch.logGroupName parameter. I don't really mind this but perhaps important to clean up the other log groups

Overall these are the values I used

    image:
      repository: public.ecr.aws/aws-observability/aws-for-fluent-bit
      tag: "2.31.2"
    cloudWatch:
      enabled: true
      logRetentionDays: 90
    firehose:
      enabled: false
    kinesis:
      enabled: false
    elasticsearch:
      enabled: false

@mhausenblas
Copy link
Member

@romogo17 thanks a bunch for that and as discussed would be great if you wanted to PR the README with a sentence to point to this (feel free to link to this issue here for "more details")?

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

No branches or pull requests

3 participants