Create shared server for `loki.source.+` network targets #3581

thepalbi · 2023-04-20T20:44:21Z

PR Description

This PR is a port of this Promtail PR: grafana/loki#9181.

Flow supports many loki.source that expose a network server, which underneath uses weaveworks commons server. When configuring this server, there's usually some boilerplate code to apply some defaults, scope the metrics under a namespace to avoid collisions, etc.

This PR creates an abstraction that we can re-use on these kind of sources.

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

CHANGELOG updated
Documentation added
Tests updated

component/common/loki/http/config.go

component/common/loki/http/server_test.go

thepalbi · 2023-04-21T20:59:13Z

@tpaschalis still have some docs to add, and some cleanup, but let me know if this goes in the right direction

tpaschalis

I think we're heading to the right direction. Looking forward to seeing how this progresses!

component/common/loki/http/config.go

component/loki/source/gcplog/internal/gcplogtarget/types.go

component/common/loki/http/server.go

thampiotr

LGMT, though I'm pretty new to this codebase.

I'm looking forward to this merging so I can incorporate the changes to my PR #3648

component/common/loki/http/config.go

component/loki/source/heroku/heroku.go

component/common/loki/http/server.go

component/loki/source/gcplog/internal/gcplogtarget/push_target.go

component/loki/source/heroku/heroku.go

thampiotr

Got a few more questions as I was working on docs for a new component.

component/loki/source/heroku/heroku.go

Got a few more questions

thepalbi · 2023-04-27T21:47:58Z

@tpaschalis @thampiotr added all settings to the config model as discussed above, and added some tests covering the river unmarshalling. Will add the docs tomorrow

docs/sources/flow/reference/components/loki.server.md

docs/sources/flow/reference/components/loki.source.gcplog.md

thepalbi · 2023-04-28T20:41:33Z

@tpaschalis @thampiotr added the docs, leaving some Qs on how to best organize them

docs/sources/flow/reference/components/loki.server.md

component/common/loki/net/config.go

docs/sources/flow/reference/components/loki.server.md

thepalbi · 2023-05-02T22:21:29Z

@thampiotr @tpaschalis should be good now

tpaschalis

LGTM, just a few documentation-related nits and we're good to go!

tpaschalis · 2023-05-03T08:13:14Z

component/common/loki/net/config.go

+	// If set, override. Don't allow a zero-value since it configure a context.WithTimeout, so the user should at least
+	// give a >0 value to it
+	if c.GracefulShutdownTimeout != 0 {
+		cfg.ServerGracefulShutdownTimeout = c.GracefulShutdownTimeout
+	}


Just to make sure, what happens when the user does not set this? Is the "30s" default applied? I'm not sure where exactly this happened.

Yes, covered by this test and applied in here that applies the defaults from the server cli flags

component/common/loki/net/config.go

tpaschalis · 2023-05-03T08:15:19Z

docs/sources/shared/flow/reference/components/loki-server-http.md

+
+ Name                   | Type       | Description                                                                                                          | Default  | Required
+------------------------|------------|----------------------------------------------------------------------------------------------------------------------|----------|----------
+ `listen_address`       | `string`   | Network address on which the server will listen for new connections. Defaults to accepting all incoming connections. | `""`     | no


Is the default here 0.0.0.0?

Nop, is empty string which has the same effect of listening to tcp conns from every incoming ip. Explained here. Either way we could display it as 0.0.0.0 on the docs, but it's explained also in the description column

tpaschalis · 2023-05-03T08:15:48Z

docs/sources/shared/flow/reference/components/loki-server-grpc.md

+
+ Name                            | Type       | Description                                                                                                          | Default      | Required
+---------------------------------|------------|----------------------------------------------------------------------------------------------------------------------|--------------|----------
+ `listen_address`                | `string`   | Network address on which the server will listen for new connections. Defaults to accepting all incoming connections. | `""`         | no


Same here, does the description mean that the default is 0.0.0.0? Or is it implied to be 0.0.0.0 by the weaveworks.Server code?

Same as above

docs/sources/flow/reference/components/loki.source.gcplog.md

docs/sources/flow/reference/components/loki.source.heroku.md

component/common/loki/net/server.go

thampiotr

LGTM! Just one rename suggestion, happy for this to merge :)

component/common/loki/net/server.go

Co-authored-by: Paschalis Tsilias <[email protected]>

thepalbi · 2023-05-03T13:30:59Z

@thampiotr @tpaschalis should be good to go now 😃

tpaschalis · 2023-05-03T16:24:20Z

Great work @thepalbi 🙌 thanks for making writing Loki components a lot easier!

(cherry picked from commit 4a3c4a7)

thepalbi requested a review from tpaschalis April 20, 2023 20:46

thepalbi commented Apr 20, 2023

View reviewed changes

component/common/loki/http/config.go Outdated Show resolved Hide resolved

component/common/loki/http/server_test.go Outdated Show resolved Hide resolved

tpaschalis reviewed Apr 24, 2023

View reviewed changes

thepalbi force-pushed the pablo/common-loki-source-http-server branch from ea4fa3a to 60f287f Compare April 24, 2023 17:30

thepalbi mentioned this pull request Apr 25, 2023

Create new http.TargetServer shared across all HTTP Promtail Targets grafana/loki#9181

Closed

5 tasks

tpaschalis mentioned this pull request Apr 26, 2023

Add loki.source.api component #3648

Merged

7 tasks

thampiotr previously approved these changes Apr 27, 2023

View reviewed changes

thampiotr reviewed Apr 27, 2023

View reviewed changes

component/loki/source/heroku/heroku.go Show resolved Hide resolved

thampiotr reviewed Apr 27, 2023

View reviewed changes

component/loki/source/heroku/heroku.go Show resolved Hide resolved

thampiotr self-requested a review April 27, 2023 13:24

thepalbi commented Apr 28, 2023

View reviewed changes

docs/sources/flow/reference/components/loki.server.md Outdated Show resolved Hide resolved

docs/sources/flow/reference/components/loki.source.gcplog.md Outdated Show resolved Hide resolved

tpaschalis reviewed May 2, 2023

View reviewed changes

docs/sources/flow/reference/components/loki.server.md Outdated Show resolved Hide resolved

tpaschalis reviewed May 2, 2023

View reviewed changes

component/common/loki/net/config.go Outdated Show resolved Hide resolved

tpaschalis reviewed May 2, 2023

View reviewed changes

docs/sources/flow/reference/components/loki.server.md Outdated Show resolved Hide resolved

thepalbi added 12 commits May 2, 2023 12:55

added commons server

15a87fd

ported loki.source.gcp

b45f5b2

migrated heroku and gcp

f82292d

added server test

dc71f63

simplfyin params

6577c4f

new river config

a9cb051

adapted both heroku and gcp

2536ee8

config tests

5000f0f

fix imports

5bcefbb

address pr comments

ffd59fb

fix tests

ad11085

fix linter

c1b90d8

thepalbi added 8 commits May 2, 2023 12:57

renaming loki/http to loki/net

cb49d8f

add tests with river raw config

172a8ea

discussed model implemented

4cab3ad

added test covering all setings

7ece9b4

added docs

6656625

pr comments and moved loki-server docs

1981d4f

new embedded docs model + loki.source.gcplog

0646d90

adjusting snippets

83db4f5

thampiotr mentioned this pull request May 2, 2023

Add prometheus.receive_http component to allow receiving metrics over HTTP #3753

Merged

4 tasks

thepalbi force-pushed the pablo/common-loki-source-http-server branch from 7a17b64 to 83db4f5 Compare May 2, 2023 16:40

thepalbi added 3 commits May 2, 2023 13:51

comments and docs fixes

deb1773

assign random grpc ports when not used

b29f96e

fix imports

9cf4269

tpaschalis approved these changes May 3, 2023

View reviewed changes

thampiotr approved these changes May 3, 2023

View reviewed changes

component/common/loki/net/server.go Outdated Show resolved Hide resolved

thepalbi and others added 5 commits May 3, 2023 09:35

Update docs/sources/flow/reference/components/loki.source.gcplog.md

3d1aad3

Co-authored-by: Paschalis Tsilias <[email protected]>

Update docs/sources/flow/reference/components/loki.source.heroku.md

6036b39

Co-authored-by: Paschalis Tsilias <[email protected]>

Update component/common/loki/net/server.go

3aaa8b3

Co-authored-by: Paschalis Tsilias <[email protected]>

docs fixes and other pr stuff

ed62713

move net package to component/common

ef385a2

tpaschalis merged commit 4a3c4a7 into main May 3, 2023

tpaschalis deleted the pablo/common-loki-source-http-server branch May 3, 2023 16:24

rfratto pushed a commit to rfratto/agent that referenced this pull request May 11, 2023

Create shared server for loki.source.+ network targets (grafana#3581)

a3195ee

(cherry picked from commit 4a3c4a7)

clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023

Create shared server for loki.source.+ network targets (#3581)

a51da80

clayton-cornell pushed a commit that referenced this pull request Aug 14, 2023

Create shared server for loki.source.+ network targets (#3581)

d635a3a

github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 27, 2024

github-actions bot locked as resolved and limited conversation to collaborators Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create shared server for `loki.source.+` network targets #3581

Create shared server for `loki.source.+` network targets #3581

thepalbi commented Apr 20, 2023 •

edited

Loading

thepalbi commented Apr 21, 2023

tpaschalis left a comment

thampiotr left a comment

thampiotr left a comment

thepalbi commented Apr 27, 2023

thepalbi commented Apr 28, 2023

thepalbi commented May 2, 2023

tpaschalis left a comment

tpaschalis May 3, 2023

thepalbi May 3, 2023

tpaschalis May 3, 2023

thepalbi May 3, 2023 •

edited

Loading

tpaschalis May 3, 2023

thepalbi May 3, 2023

thampiotr left a comment

thepalbi commented May 3, 2023

tpaschalis commented May 3, 2023

Create shared server for loki.source.+ network targets #3581

Create shared server for loki.source.+ network targets #3581

Conversation

thepalbi commented Apr 20, 2023 • edited Loading

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

thepalbi commented Apr 21, 2023

tpaschalis left a comment

Choose a reason for hiding this comment

thampiotr left a comment

Choose a reason for hiding this comment

thampiotr left a comment

Choose a reason for hiding this comment

thepalbi commented Apr 27, 2023

thepalbi commented Apr 28, 2023

thepalbi commented May 2, 2023

tpaschalis left a comment

Choose a reason for hiding this comment

tpaschalis May 3, 2023

Choose a reason for hiding this comment

thepalbi May 3, 2023

Choose a reason for hiding this comment

tpaschalis May 3, 2023

Choose a reason for hiding this comment

thepalbi May 3, 2023 • edited Loading

Choose a reason for hiding this comment

tpaschalis May 3, 2023

Choose a reason for hiding this comment

thepalbi May 3, 2023

Choose a reason for hiding this comment

thampiotr left a comment

Choose a reason for hiding this comment

thepalbi commented May 3, 2023

tpaschalis commented May 3, 2023

Create shared server for `loki.source.+` network targets #3581

Create shared server for `loki.source.+` network targets #3581

thepalbi commented Apr 20, 2023 •

edited

Loading

thepalbi May 3, 2023 •

edited

Loading