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

[BUG] Log Links should work with CloudWatch FluentD Out Of the Box #2635

Open
2 tasks done
Tracked by #2917
EngHabu opened this issue Jun 28, 2022 · 11 comments · May be fixed by flyteorg/flyteplugins#293
Open
2 tasks done
Tracked by #2917

[BUG] Log Links should work with CloudWatch FluentD Out Of the Box #2635

EngHabu opened this issue Jun 28, 2022 · 11 comments · May be fixed by flyteorg/flyteplugins#293
Labels
bug Something isn't working good first issue Good for newcomers infrastructure stale
Milestone

Comments

@EngHabu
Copy link
Contributor

EngHabu commented Jun 28, 2022

Describe the bug

As it stands, users wishing to enable CloudWatch logs will need to go through the AWS Process to do so here and then manually modify the fluent-bit-config configmap to alter the generated log stream to match what Flyte expects.

kubectl edit cm -n amazon-cloudwatch fluent-bit-config 
[OUTPUT]
    Name                cloudwatch_logs
    Match               application.*
    region              ${AWS_REGION}
    log_group_name      /aws/containerinsights/${CLUSTER_NAME}/application
    log_stream_prefix   var.
    auto_create_group   true
    extra_user_agent    container-insights

And then use this configmap for flyte:

  task_logs.yaml: |
    plugins:
      logs:
        cloudwatch-template-uri: 'https://{vars.region}.console.aws.amazon.com/cloudwatch/home?region={vars.region}#logsV2:log-groups/log-group/$252Faws$252Fcontainerinsights$252F<log group name>$252Fapplication$3FlogStreamNameFilter$3Dvar.application.var.log.containers.{{ .podName }}_{{ .namespace }}_{{ .containerName }}'

We should change the default template defined here to be: https://console.aws.amazon.com/cloudwatch/home?region=%s#logsV2:log-groups/log-group/%s$3FlogStreamNameFilter=var.log.containers.{{ .podName }}_{{ .namespace }}_{{ .containerName }}

Expected behavior

  1. Follow AWS Guide to deploy FluentD: https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Container-Insights-setup-logs-FluentBit.html
  2. Enable CloudWatch logs in Flyte:
      task_logs.yaml: |
        plugins:
          logs:
            cloudwatch-enabled: true
            cloudwatch-log-group: 'bv-ml-pipelines'
            cloudwatch-region: 'us-east-1'
            kubernetes-enabled: false
    
  3. SUCCESS ✔️

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@EngHabu EngHabu added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Jun 28, 2022
@wild-endeavor wild-endeavor added infrastructure and removed untriaged This issues has not yet been looked at by the Maintainers labels Jul 22, 2022
@wild-endeavor wild-endeavor added this to the 1.2.0-candidate milestone Jul 22, 2022
@wild-endeavor wild-endeavor added the good first issue Good for newcomers label Jul 22, 2022
@Azanul
Copy link

Azanul commented Sep 30, 2022

I'd like to work on this.

@Azanul
Copy link

Azanul commented Sep 30, 2022

#take

@Azanul
Copy link

Azanul commented Sep 30, 2022

#self-assign

@Azanul
Copy link

Azanul commented Oct 2, 2022

@samhita-alla here

@eapolinario eapolinario modified the milestones: 1.2.0-candidate, 1.3.0 Oct 3, 2022
@Azanul Azanul removed their assignment Oct 8, 2022
@Azanul
Copy link

Azanul commented Oct 15, 2022

#self-assign

@Smartmind12
Copy link
Contributor

@samhita-alla Kindly review the PR and let me know if any changes are required...Thanks!

@Azanul Azanul linked a pull request Oct 23, 2022 that will close this issue
8 tasks
@andrusha
Copy link

andrusha commented Nov 25, 2022

This assumes that the cluster is going to be deployed with FluentBit by-default, while FluentD is valid and recommended option, to which the older configuration is pointing to. This change will not be applicable for the all users.

https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Container-Insights-setup-logs-FluentBit.html

@EngHabu
Copy link
Contributor Author

EngHabu commented Nov 26, 2022

@andrusha do you mind elaborating on this? Are you commenting on the discussion we are having on the PR? flyteorg/flyteplugins#293 (comment)

@andrusha
Copy link

andrusha commented Nov 26, 2022 via email

@EngHabu
Copy link
Contributor Author

EngHabu commented Nov 26, 2022

Aha, I understand, I agree changing the default can break other people, do you know for a fact that FlutentBit in compatibility mode works out of the box? I vaguely recall hostname was still a requirement though, no?

Perhaps a better approach is to:

  1. Still add support for a host name template parameter
  2. Instead of changing the default, add another CloudWatchV2Enabled flag that default to the new version format...
  3. Add a "Enabling Cloud Provider-Native logs" where we can elaborate on templating for URLs, StackDriver, CloudWatch, FluentD and FluentBit.

@cosmicBboy cosmicBboy modified the milestones: 1.3.0, 2023 Q1 Backlog Jan 25, 2023
@Azanul Azanul removed their assignment Sep 19, 2023
Copy link

Hello 👋, this issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will engage on it to decide if it is still applicable.
Thank you for your contribution and understanding! 🙏

@github-actions github-actions bot added the stale label Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers infrastructure stale
Projects
None yet
8 participants