lambda-promtail: Add support for Kinesis data stream events #5977

juissi-t · 2022-04-21T05:53:21Z

What this PR does / why we need it:

This PR adds support for sending Kinesis data stream events to lambda-promtail. One use case would be e.g. to send CloudFront realtime logs to Loki.

Which issue(s) this PR fixes:

Fixes #5978

Special notes for your reviewer:

n/a

Checklist

Documentation added
Tests updated
Is this an important fix or new feature? Add an entry in the CHANGELOG.md.
Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

juissi-t · 2022-05-03T09:20:22Z

@grafana/loki-team: could someone review this PR, please?

juissi-t · 2022-05-11T06:00:07Z

@KMiller-Grafana this PR has been open for about three weeks now. Any chance you could take a look at it?

slim-bean · 2022-05-11T09:08:27Z

Apologies @juissi-t, it's been busy trying to finish up projects at the end of our quarter and this week most of the team is at an office.

Three weeks is a long time :( very sorry about that, we will take a look at this as soon as we can, but may not be until t week.

In the meantime if you could fix the changelog conflict that will help us get this merged as soon as we can

juissi-t · 2022-05-11T16:20:47Z

Thanks, it would be great if you could review this in the near future!

In the meantime if you could fix the changelog conflict that will help us get this merged as soon as we can

Rebased the PR.

grafanabot · 2022-05-11T16:23:49Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

KMiller-Grafana

I approve of the docs portion of this PR.

juissi-t · 2022-06-16T12:56:23Z

@slim-bean any chance you could take a look at this, now that it's been over a month since the last time I reached out. 🙏

grafanabot · 2022-06-23T07:36:15Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-06-23T07:36:55Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

juissi-t · 2022-08-01T09:11:02Z

@cstyan I rebased the PR. Some test seems to fail, but to me it looks unrelated to this change. Could you check this PR?

luva03 · 2022-08-08T08:57:29Z

@juissi-t @slim-bean @cstyan

Is there any progress on this PR? We are very interested in using this feature. 🙏 TY

juissi-t · 2022-08-17T10:22:52Z

Is there any progress on this PR? We are very interested in using this feature. pray TY

I have been trying to ping the people who have commented or assigned this, but no luck yet. I'll keep trying.

@slim-bean @cstyan could someone take a look? This has been open for over three months now.

grafanabot · 2022-08-17T10:31:35Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

juissi-t · 2022-08-26T07:06:48Z

Pinging recent contributors to lambda-promtail: @dannykopping @jonathanmorais @simonswine. Could one of you maybe take a look at this pull request? Or find someone who could do that?

This PR has been open for over four months now, and I find it quite disheartening for any new contributor to an open source project backed by a company to have to wait so long for a review for a fairly trivial new feature. I know Loki team has its priorities and for sure you are busy, but still... If you don't want to have this, that's fine too, but could someone at least say something?

dannykopping

@juissi-t big apologies from the Loki team - we've dropped the ball here.

We are quite swamped as you can imagine, but in the future please feel free to reach out in public Slack if you're not getting any response. GitHub notifications are notoriously unreliable, and sometimes we miss things, sometimes we simply don't have the time.

All that being said, thank you for your contribution. I have reviewed it and have one comment, and also I'd ask you to add a test for this please.

tools/lambda-promtail/lambda-promtail/kinesis.go

grafanabot · 2022-08-26T08:36:42Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

dannykopping

Thanks for the changes.
Added another comment

Sidenote:
Please don't rebase once a review begins as it rewrites history and it becomes difficult to do incremental reviews in GitHub

tools/lambda-promtail/lambda-promtail/kinesis_test.go

dannykopping

LGTM, thanks for the contribution @juissi-t
Apologies again for the delays here, I hope it doesn't deter you from contributing again

grafanabot · 2022-08-26T11:23:23Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

juissi-t · 2022-08-26T11:34:19Z

Apologies again for the delays here, I hope it doesn't deter you from contributing again

Thanks a lot for your help in getting this reviewed @dannykopping! I will keep in mind the option of asking in Slack if I find another scratch I need to itch, and the PR review seems to take too long. 😄

…5977) **What this PR does / why we need it**: This PR adds support for sending Kinesis data stream events to lambda-promtail. One use case would be e.g. to send CloudFront realtime logs to Loki. **Which issue(s) this PR fixes**: Fixes grafana#5978 **Special notes for your reviewer**: n/a **Checklist** - [x] Documentation added - [x] Tests updated - [x] Is this an important fix or new feature? Add an entry in the `CHANGELOG.md`. - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md`

**What this PR does / why we need it**: #5977 With the addition of the kinesis data stream function add kinesis data stream to use in terraform

…#7632) **What this PR does / why we need it**: grafana#5977 With the addition of the kinesis data stream function add kinesis data stream to use in terraform

juissi-t requested review from KMiller-Grafana and a team as code owners April 21, 2022 05:53

pull-request-size bot added the size/M label Apr 21, 2022

juissi-t force-pushed the main branch from f0bb3db to d974827 Compare April 21, 2022 05:55

juissi-t mentioned this pull request Apr 21, 2022

lambda-promtail: Allow ingesting real-time logs from Cloudfront #5978

Closed

juissi-t force-pushed the main branch from d974827 to 2281501 Compare May 3, 2022 09:19

juissi-t force-pushed the main branch from 2281501 to 04dfdfb Compare May 11, 2022 16:19

github-actions bot added the area/docs label May 11, 2022

KMiller-Grafana approved these changes May 20, 2022

View reviewed changes

chaudum removed the type/docs label Jun 13, 2022

cstyan self-assigned this Jun 22, 2022

juissi-t force-pushed the main branch 3 times, most recently from 6e0b1db to 5f21387 Compare June 23, 2022 07:32

juissi-t force-pushed the main branch from 5f21387 to 78f90d8 Compare August 1, 2022 08:57

juissi-t force-pushed the main branch from 78f90d8 to b46a137 Compare August 17, 2022 10:25

github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Aug 17, 2022

dannykopping reviewed Aug 26, 2022

View reviewed changes

tools/lambda-promtail/lambda-promtail/kinesis.go Show resolved Hide resolved

juissi-t added 2 commits August 26, 2022 10:25

lambda-promtail: Add support for Kinesis data stream events

df57dc6

Check if eve is nil; add test for kinesis

40d8182

juissi-t force-pushed the main branch from b46a137 to 40d8182 Compare August 26, 2022 08:31

pull-request-size bot added size/L and removed size/M labels Aug 26, 2022

dannykopping reviewed Aug 26, 2022

View reviewed changes

tools/lambda-promtail/lambda-promtail/kinesis_test.go Show resolved Hide resolved

Test that labels are correct

e865b43

dannykopping approved these changes Aug 26, 2022

View reviewed changes

dannykopping merged commit 073c620 into grafana:main Aug 26, 2022

changhyuni mentioned this pull request Sep 25, 2022

lambda-promtail: Add kinesis data stream to use in terraform #7239

Closed

5 tasks

osg-grafana removed the area/docs label Oct 19, 2022

changhyuni mentioned this pull request Nov 8, 2022

lambda-promtail: Add kinesis data stream to use in terraform #7632

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lambda-promtail: Add support for Kinesis data stream events #5977

lambda-promtail: Add support for Kinesis data stream events #5977

juissi-t commented Apr 21, 2022 •

edited

juissi-t commented May 3, 2022

juissi-t commented May 11, 2022

slim-bean commented May 11, 2022

juissi-t commented May 11, 2022

grafanabot commented May 11, 2022

KMiller-Grafana left a comment

juissi-t commented Jun 16, 2022

grafanabot commented Jun 23, 2022

grafanabot commented Jun 23, 2022

juissi-t commented Aug 1, 2022

luva03 commented Aug 8, 2022 •

edited

juissi-t commented Aug 17, 2022

grafanabot commented Aug 17, 2022

juissi-t commented Aug 26, 2022

dannykopping left a comment

grafanabot commented Aug 26, 2022

dannykopping left a comment •

edited

dannykopping left a comment

grafanabot commented Aug 26, 2022

juissi-t commented Aug 26, 2022

lambda-promtail: Add support for Kinesis data stream events #5977

lambda-promtail: Add support for Kinesis data stream events #5977

Conversation

juissi-t commented Apr 21, 2022 • edited

juissi-t commented May 3, 2022

juissi-t commented May 11, 2022

slim-bean commented May 11, 2022

juissi-t commented May 11, 2022

grafanabot commented May 11, 2022

KMiller-Grafana left a comment

Choose a reason for hiding this comment

juissi-t commented Jun 16, 2022

grafanabot commented Jun 23, 2022

grafanabot commented Jun 23, 2022

juissi-t commented Aug 1, 2022

luva03 commented Aug 8, 2022 • edited

juissi-t commented Aug 17, 2022

grafanabot commented Aug 17, 2022

juissi-t commented Aug 26, 2022

dannykopping left a comment

Choose a reason for hiding this comment

grafanabot commented Aug 26, 2022

dannykopping left a comment • edited

Choose a reason for hiding this comment

dannykopping left a comment

Choose a reason for hiding this comment

grafanabot commented Aug 26, 2022

juissi-t commented Aug 26, 2022

juissi-t commented Apr 21, 2022 •

edited

luva03 commented Aug 8, 2022 •

edited

dannykopping left a comment •

edited