Targets: Add Heroku HTTPS drain target #6448

thepalbi · 2022-06-21T17:22:29Z

What this PR does / why we need it:
Heroku streams logs from all its service layers (systems, builds, application) to a common component called LogPlex, from which they can be consumed. In the Heroku platform itself, the ways for consuming logs are either via CLI, which offers basic filtering and tail capabilities; or in their web UI.

Apart from both of these, LogPlex has a concept of drains, which are destinations where logs can be streamed to, and shipped, consumed, etc. Between these, once exposes a mechanism for shipping to a HTTPS destination. This is called HTTPS Drain.

This PR adds a new promtail target that exposes an HTTP server (in the form of a drain as described above), that can be used to receive logs from Heroku.

The target allows the following configurations:

  heroku_drain:
    server:
      http_listen_address: 0.0.0.0
      http_listen_port: 8080
      log_level: debug
      register_instrumentation: true
    labels: 
      job: heroku_logs_test
    use_incoming_timestamp: true

This target exposes some of the received "label" as "internal labels" to be re-labeled by the user using the scrape config (much like the syslog target does), and dropped if not used. These are:

__logplex_host: hostname of the log producer
__logplex_app: application name of the log producer
__logplex_proc: process id of the log producer
__logplex_log_id: log identifier

On the other hand, Heroku offers best-effort delivery guarantees. That said, this target implementation does not offer no other guarantees. In case that some delivery from Heroku to promtail fails, this will be dropped and eventually Heroku will notify this receiver that some log entries were dropped.

Which issue(s) this PR fixes:
n/a

Special notes for your reviewer:
Since Heroku ships their metrics through HTTP, in a syslog formatted message with octet framing (just like promtail's syslog target expects) this new target is very similar to both the loki-push and syslog targets.

Also, in the Heroku HTTPS drain docs, a mention is made that the syslog format that Heroku uses is not RFC-compliant. Because of that, an utils Go lib that the same company has publicly available is used for parsing each entry.

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

CLAassistant · 2022-06-21T17:22:36Z

All committers have signed the CLA.

thepalbi · 2022-06-21T17:25:58Z

Happy to add docs, don't know in which specific section to do so

grafanabot · 2022-06-21T18:17:32Z

./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

This looks great @thepalbi, thanks for the contribution!

The docs can be added in these sections:
https://grafana.com/docs/loki/latest/clients/promtail/configuration/#scrape_configs and https://grafana.com/docs/loki/latest/clients/promtail/scraping/

which is defined here docs/sources/clients/promtail/configuration.md and docs/sources/clients/promtail/scraping.md respectively.

I've added a couple nits but overall this looks solid.

I think we'll need to get the terminology consistent here as well, for clarity. We're referring to "heroku", "heroku target", "heroku drain" in a few places... it'd be great if we could standardise on "heroku drain", or whatever @KMiller-Grafana would suggest.

clients/pkg/promtail/targets/heroku/metrics.go

clients/pkg/promtail/targets/heroku/target.go

Co-authored-by: Danny Kopping <dannykopping@gmail.com>

grafanabot · 2022-06-29T19:21:46Z

./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.4%
+               loki	0%

thepalbi · 2022-06-29T23:45:21Z

I think we'll need to get the terminology consistent here as well, for clarity. We're referring to "heroku", "heroku target", "heroku drain" in a few places... it'd be great if we could standardise on "heroku drain", or whatever @KMiller-Grafana would suggest.

Completely agree, I'll do a cleanup pass making the naming consistent with "heroku drain" since it's the de-facto naming for this mechanism for moving heroku logs

In some places I'll compose the naming like: heroku_drain_targer_* to keep naming consistent with other targets.

Also, modifying internal labels produced by the target to:

		lb.Set("__heroku_drain_host", message.Hostname)
		lb.Set("__heroku_drain_app", message.Application)
		lb.Set("__heroku_drain_proc", message.Process)
		lb.Set("__heroku_drain_log_id", message.ID)

to keep those names consistent with the target.

grafanabot · 2022-06-30T00:02:56Z

./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 · 2022-06-30T07:35:06Z

Thanks for the changes @thepalbi! This is almost ready for approval; can you please address the last couple comments? I'll then re-review

thepalbi · 2022-06-30T14:27:14Z

@dannykopping sure thing Danny! Should I address the documentation in a follow up PR?

grafanabot · 2022-06-30T15:52:18Z

./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.3%
+            querier	0%
+ querier/queryrange	0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

dannykopping · 2022-07-04T08:30:32Z

@dannykopping sure thing Danny! Should I address the documentation in a follow up PR?

~~If you could include it in this one, I think that'd be preferable. Thanks! 🙏~~
Ah I see you've already created #6560 - no biggie 👍

thepalbi · 2022-07-04T15:04:48Z

@dannykopping added docs in this PR as discussed!

grafanabot · 2022-07-04T15:29:47Z

./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.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

dannykopping

Approving the code 👍 great work @thepalbi!

@KMiller-Grafana will review the docs, and then we can merge

KMiller-Grafana

I've made lots of small suggestions to improve the capitalization and grammar. Thanks for the thorough documentation.

docs/sources/clients/promtail/configuration.md

docs/sources/clients/promtail/scraping.md

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

thepalbi · 2022-07-14T15:47:10Z

@KMiller-Grafana just applied all your suggestions, and removed something that I copied erroneously form Promtail's GCP docs

grafanabot · 2022-07-14T15:50:45Z

./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-07-14T16:18:06Z

./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

Docs look good to me. Thank you for the changes.

Add heroku target

89162cb

thepalbi requested a review from a team as a code owner June 21, 2022 17:22

pull-request-size bot added the size/XL label Jun 21, 2022

Fix imports order

53b3673

thepalbi added 2 commits June 21, 2022 15:01

Fix typo

1641b46

Fix imports order

1d2e411

dannykopping reviewed Jun 29, 2022

View reviewed changes

thepalbi and others added 2 commits June 29, 2022 16:16

Update clients/pkg/promtail/targets/heroku/metrics.go

8b48258

Co-authored-by: Danny Kopping <dannykopping@gmail.com>

Update clients/pkg/promtail/targets/heroku/metrics.go

5a9b31e

Co-authored-by: Danny Kopping <dannykopping@gmail.com>

thepalbi added 2 commits June 29, 2022 20:38

replace logger with much simpler setup

da6b260

wrapping logger with component name

100ad27

thepalbi added 2 commits June 29, 2022 20:55

make naming standard accross all changes + drivebys

66d1253

import order

6fde88e

fail fast on not prom compatible job name

3297a6a

thepalbi mentioned this pull request Jun 30, 2022

Docs: Add Heroku Drain target documentation #6560

Closed

4 tasks

thepalbi added 3 commits July 4, 2022 11:30

fail fast on mergo error

079da42

First docs iteration

73df050

fix typo introduced

489bb7f

thepalbi requested a review from KMiller-Grafana as a code owner July 4, 2022 15:03

github-actions bot added the area/docs label Jul 4, 2022

Fix imports order

e0af49b

dannykopping approved these changes Jul 4, 2022

View reviewed changes

KMiller-Grafana added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Jul 13, 2022

KMiller-Grafana suggested changes Jul 13, 2022

View reviewed changes

thepalbi and others added 9 commits July 14, 2022 11:21

Update docs/sources/clients/promtail/configuration.md

f2b8c78

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

Update docs/sources/clients/promtail/scraping.md

63ed13e

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

Update docs/sources/clients/promtail/configuration.md

d52f09c

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

Update docs/sources/clients/promtail/configuration.md

b3ebaa5

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

Update docs/sources/clients/promtail/configuration.md

6980687

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

Update docs/sources/clients/promtail/configuration.md

ea85802

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

Update docs/sources/clients/promtail/scraping.md

9ecf950

Co-authored-by: Karen Miller <84039272+KMiller-Grafana@users.noreply.github.com>

remove things that were from gcp docs

a92aa66

Merge branch 'main' into pablo/heroku-target-onecommit

27eb4c4

fix go mod check

42b5fd8

KMiller-Grafana approved these changes Jul 14, 2022

View reviewed changes

dannykopping merged commit 0e28452 into grafana:main Jul 15, 2022

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

lxwzy pushed a commit to lxwzy/loki that referenced this pull request Nov 7, 2022

Targets: Add Heroku HTTPS drain target (grafana#6448)

d0a6394

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Targets: Add Heroku HTTPS drain target #6448

Targets: Add Heroku HTTPS drain target #6448

thepalbi commented Jun 21, 2022

CLAassistant commented Jun 21, 2022 •

edited

Loading

thepalbi commented Jun 21, 2022

grafanabot commented Jun 21, 2022

dannykopping left a comment

grafanabot commented Jun 29, 2022

thepalbi commented Jun 29, 2022 •

edited

Loading

grafanabot commented Jun 30, 2022

dannykopping commented Jun 30, 2022

thepalbi commented Jun 30, 2022

grafanabot commented Jun 30, 2022

dannykopping commented Jul 4, 2022 •

edited

Loading

thepalbi commented Jul 4, 2022

grafanabot commented Jul 4, 2022

dannykopping left a comment

KMiller-Grafana left a comment

thepalbi commented Jul 14, 2022

grafanabot commented Jul 14, 2022

grafanabot commented Jul 14, 2022

KMiller-Grafana left a comment

Targets: Add Heroku HTTPS drain target #6448

Targets: Add Heroku HTTPS drain target #6448

Conversation

thepalbi commented Jun 21, 2022

CLAassistant commented Jun 21, 2022 • edited Loading

thepalbi commented Jun 21, 2022

grafanabot commented Jun 21, 2022

dannykopping left a comment

Choose a reason for hiding this comment

grafanabot commented Jun 29, 2022

thepalbi commented Jun 29, 2022 • edited Loading

grafanabot commented Jun 30, 2022

dannykopping commented Jun 30, 2022

thepalbi commented Jun 30, 2022

grafanabot commented Jun 30, 2022

dannykopping commented Jul 4, 2022 • edited Loading

thepalbi commented Jul 4, 2022

grafanabot commented Jul 4, 2022

dannykopping left a comment

Choose a reason for hiding this comment

KMiller-Grafana left a comment

Choose a reason for hiding this comment

thepalbi commented Jul 14, 2022

grafanabot commented Jul 14, 2022

grafanabot commented Jul 14, 2022

KMiller-Grafana left a comment

Choose a reason for hiding this comment

CLAassistant commented Jun 21, 2022 •

edited

Loading

thepalbi commented Jun 29, 2022 •

edited

Loading

dannykopping commented Jul 4, 2022 •

edited

Loading