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

feat: lambda-promtail; ensure messages to Kinesis are usable by refactoring parsing of KinesisEvent to match parsing of CWEvents + code cleanup #13098

Merged

Conversation

HatiCode
Copy link
Contributor

@HatiCode HatiCode commented Jun 2, 2024

Refactor parsing of KinesisEvent to match parsing of CWEvents + code cleanup

What this PR does / why we need it:
This PR refactors the kinesis.go parsing code to match the way CWEvents are sent to Promtail.
When Kinesis is chosen as a delivery method it should still have a simple and usable log format. So far the message in the DATA_MESSAGE is unusable. This is solved by assigning labels to the logEvents in the DATA_MESSAGE the same way it's done in cw.go

Which issue(s) this PR fixes:
Fixes #10544

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@HatiCode HatiCode requested a review from a team as a code owner June 2, 2024 14:36
@CLAassistant
Copy link

CLAassistant commented Jun 2, 2024

CLA assistant check
All committers have signed the CLA.

@cstyan
Copy link
Contributor

cstyan commented Jun 5, 2024

hey @HatiCode there are some lambda promtail tests failing

@HatiCode
Copy link
Contributor Author

HatiCode commented Jul 2, 2024

hey @HatiCode there are some lambda promtail tests failing

Fixed. @cstyan

@cstyan cstyan changed the title Refactor parsing of KinesisEvent to match parsing of CWEvents + code cleanup [feat] lamda-promtail; ensure messages to Kinesis are usable by refactoring parsing of KinesisEvent to match parsing of CWEvents + code cleanup Jul 5, 2024
@cstyan cstyan changed the title [feat] lamda-promtail; ensure messages to Kinesis are usable by refactoring parsing of KinesisEvent to match parsing of CWEvents + code cleanup feat: lambda-promtail; ensure messages to Kinesis are usable by refactoring parsing of KinesisEvent to match parsing of CWEvents + code cleanup Jul 5, 2024
Comment on lines +93 to +99
model.LabelName("__aws_cloudwatch_log_group"): model.LabelValue(recordData.LogGroup),
model.LabelName("__aws_cloudwatch_owner"): model.LabelValue(recordData.Owner),
}

if keepStream {
labels[model.LabelName("__aws_cloudwatch_log_stream")] = model.LabelValue(recordData.LogStream)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should these not be prefixed with __aws_kinesis instead of __aws_cloudwatch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cstyan depending on the pov. The values are coming from cloudwatch, not from the K stream. K is a data stream not the source so I believe you want to have a filter that's a reflection of what your actual cloudwatch looks like, not the tool you used in the middle to stream that data.

Copy link
Contributor

Choose a reason for hiding this comment

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

that makes sense, thanks for the context 👍

@cstyan cstyan merged commit dbfb19b into grafana:main Jul 19, 2024
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review labels consistency in lambda-promtail
3 participants