Skip to content

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

Merged
cstyan merged 7 commits into
grafana:mainfrom
HatiCode:fix/align-kinesis-log-parsing-with-cw-log-parsing
Jul 19, 2024
Merged

feat: lambda-promtail; ensure messages to Kinesis are usable by refactoring parsing of KinesisEvent to match parsing of CWEvents + code cleanup#13098
cstyan merged 7 commits into
grafana:mainfrom
HatiCode:fix/align-kinesis-log-parsing-with-cw-log-parsing

Conversation

@HatiCode

@HatiCode HatiCode commented Jun 2, 2024

Copy link
Copy Markdown
Contributor

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

CLAassistant commented Jun 2, 2024

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cstyan

cstyan commented Jun 5, 2024

Copy link
Copy Markdown
Contributor

hey @HatiCode there are some lambda promtail tests failing

@HatiCode

HatiCode commented Jul 2, 2024

Copy link
Copy Markdown
Contributor Author

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 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 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants