fix: crash when updating import.http config #2204

kinolaev · 2024-12-02T18:41:28Z

PR Description

This PR fixes a crash when updating import.http config that occurred because importsource.HTTPArguments was passed to component/remote/http.Update function instead of component/remote/http.Arguments.

Crash log:

ts=2024-12-02T16:19:30.25946014Z level=info msg="reload requested via /-/reload endpoint" service=http
ts=2024-12-02T16:19:30.260271407Z level=info msg="starting complete graph evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd
ts=2024-12-02T16:19:30.260547266Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=prometheus.remote_write.default duration=210.501µs
ts=2024-12-02T16:19:30.260684586Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=local.file_match.txlogs duration=90.358µs
ts=2024-12-02T16:19:30.261002744Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=loki.relabel.journal duration=258.232µs
ts=2024-12-02T16:19:30.261154033Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=labelstore duration=80.823µs
ts=2024-12-02T16:19:30.261220731Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=otel duration=32.895µs
ts=2024-12-02T16:19:30.261354245Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=loki.write.default duration=107.018µs
ts=2024-12-02T16:19:30.261517394Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=loki.source.journal.read duration=101.364µs
ts=2024-12-02T16:19:30.261699889Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=loki.relabel.host duration=127.935µs
ts=2024-12-02T16:19:30.261816661Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=tracing duration=47.051µs
ts=2024-12-02T16:19:30.312654144Z level=info msg="finished node evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd node_id=import.http.tx duration=50.799132ms
ts=2024-12-02T16:19:30.31278222Z level=info msg="finished complete graph evaluation" controller_path=/ controller_id="" trace_id=f5a86bef2cca442f28cd0b9c6ecf3ddd duration=52.844717ms
2024/12/02 19:19:30 http: panic serving 127.0.0.1:56724: interface conversion: component.Arguments is importsource.HTTPArguments, not http.Arguments
goroutine 22012 [running]:
net/http.(*conn).serve.func1()
        /usr/local/go/src/net/http/server.go:1903 +0xbe
panic({0x9501e00?, 0xc003f1a4e0?})
        /usr/local/go/src/runtime/panic.go:770 +0x132
github.com/grafana/alloy/internal/component/remote/http.(*Component).Update(0xc0036c6488, {0xa329800?, 0xc00354dda8?})
        /drone/src/internal/component/remote/http/http.go:269 +0x457
github.com/grafana/alloy/internal/runtime/internal/importsource.(*ImportHTTP).Evaluate(0xc00254b6c0, 0xc0070df0d0)
        /drone/src/internal/runtime/internal/importsource/import_http.go:91 +0x3d5
github.com/grafana/alloy/internal/runtime/internal/controller.(*ImportConfigNode).Evaluate(0xc0035e4000, 0x98ffbe0?)
        /drone/src/internal/runtime/internal/controller/node_config_import.go:180 +0x2e
github.com/grafana/alloy/internal/runtime/internal/controller.(*Loader).evaluate(0xc00254b1e0, {0xbffbd00, 0xc005dd2f00}, {0xc0b2158, 0xc0035e4000})
        /drone/src/internal/runtime/internal/controller/loader.go:823 +0x49
github.com/grafana/alloy/internal/runtime/internal/controller.(*Loader).Apply.func2({0xbffe540, 0xc0035e4000})
        /drone/src/internal/runtime/internal/controller/loader.go:237 +0xad8
github.com/grafana/alloy/internal/runtime/internal/dag.WalkTopological(0xc0033d82a0, {0xc003302d00, 0xc, 0x117d2d50?}, 0xc00354eca0)
        /drone/src/internal/runtime/internal/dag/walk.go:83 +0x222
github.com/grafana/alloy/internal/runtime/internal/controller.(*Loader).Apply(0xc00254b1e0, {0x0, {0xc002d05180, 0xa, 0x10}, {0xc0070de7d0, 0x2, 0x2}, {0x0, 0x0, ...}, ...})
        /drone/src/internal/runtime/internal/controller/loader.go:188 +0xb6d
github.com/grafana/alloy/internal/runtime.(*Runtime).applyLoaderConfig(0xc003269500, {0x0, {0xc002d05180, 0xa, 0x10}, {0xc0070de7d0, 0x2, 0x2}, {0x0, 0x0, ...}, ...})
        /drone/src/internal/runtime/alloy.go:334 +0xd8
github.com/grafana/alloy/internal/runtime.(*Runtime).LoadSource(0xc003269500, 0xc002456a10, 0x0, {0x7ffc3e40de99, 0x17})
        /drone/src/internal/runtime/alloy.go:307 +0x365
github.com/grafana/alloy/internal/alloycli.(*alloyRun).Run.func5()
        /drone/src/internal/alloycli/cmd_run.go:359 +0x285
github.com/grafana/alloy/internal/alloycli.(*alloyRun).Run.func4()
        /drone/src/internal/alloycli/cmd_run.go:296 +0x16
github.com/grafana/alloy/internal/service/http.(*Service).Run.func3({0xc0951d0, 0xc0058606c0}, 0x0?)
        /drone/src/internal/service/http/http.go:215 +0xbe
net/http.HandlerFunc.ServeHTTP(0xc0886a0?, {0xc0951d0?, 0xc0058606c0?}, 0xa8ec140?)
        /usr/local/go/src/net/http/server.go:2171 +0x29
go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux.traceware.ServeHTTP({{0xa8d683d, 0x5}, {0xc029950, 0xc002c920a0}, {0xc089f00, 0xc002734cc0}, {0xbffb840, 0xc0024a04d0}, 0xb07f710, 0x0, ...}, ...)
        /go/pkg/mod/go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/[email protected]/mux.go:179 +0xbb2
github.com/gorilla/mux.(*Router).ServeHTTP(0xc002cb6000, {0xc0886a0, 0xc006050540}, 0xc004392b40)
        /go/pkg/mod/github.com/gorilla/[email protected]/mux.go:212 +0x1e2
golang.org/x/net/http2/h2c.h2cHandler.ServeHTTP({{0xbffc200?, 0xc002cb6000?}, 0xc00320ff10?}, {0xc0886a0, 0xc006050540}, 0xc004392b40)
        /go/pkg/mod/golang.org/x/[email protected]/http2/h2c/h2c.go:125 +0x697
net/http.serverHandler.ServeHTTP({0xc00292cae0?}, {0xc0886a0?, 0xc006050540?}, 0x6?)
        /usr/local/go/src/net/http/server.go:3142 +0x8e
net/http.(*conn).serve(0xc00443c7e0, {0xc0b1150, 0xc002a7a510})
        /usr/local/go/src/net/http/server.go:2044 +0x5e8
created by net/http.(*Server).Serve in goroutine 626
        /usr/local/go/src/net/http/server.go:3290 +0x4b4

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

CHANGELOG.md updated
Documentation added
Tests updated
Config converters updated

CLAassistant · 2024-12-02T18:41:36Z

All committers have signed the CLA.

CHANGELOG.md

Co-authored-by: Paulin Todev <[email protected]>

kinolaev · 2024-12-02T20:13:05Z

Thanks @ptodev ! I added a commit with your suggestion, do I need to squash two commits into one?

ptodev · 2024-12-02T20:25:16Z

do I need to squash two commits into one?

@kinolaev No, no need. Thank you for your contribution! I will merge it when the CI completes.

kinolaev · 2024-12-03T07:00:13Z

@ptodev , it seems like the errors in the failed test are unrelated to the PR, because the test also fails on main branch. I checked it by executing command CGO_LDFLAGS="-ld_classic $CGO_LDFLAGS" make GO_TAGS="nodocker" test from github workflow locally.

* fix: crash when updating import.http config

* Fixed an issue in the `otlp.exporter.prometheus` component (#2102) * Fixed an issue in the `otlp.exporter.prometheus` component * Fixed an issue in the `otlp.exporter.prometheus` component * Fix potential deadlock in import statements (#2129) * Fix potential deadlock in import statements * change * typo * fix: race condition UI remotecfg (#2160) * Refactor ui remtoecfg components to avoid race condition * Fix accidental cast to pointer that should have been struct * Update changelog * fix: fully prevent panic in remotecfg ui (#2164) * Fully prevent panic in remotecfg ui * Address PR feedback * Fix deadlock due to infinite retry (#2174) * Fix deadlock due to infinite retry * changelog * Update ckit to fix memberlist logging issues (#2186) * Upgrade ckit and changelog * go mod tidy * `loki.source.podlogs`: Fix issue which disables clustering unintentionally. (#2187) * Fix issue which disables clustering unintentionally. * prometheus.operator.*: allow setting informer_sync_timeout (#2161) * prometheus.operator.*: allow setting informer_sync_timeout * default to 1m * docs * fix(pyroscope): allow slashes in tag name (#2172) * loki.source.podlogs: For clustering only take into account some labels (#2190) * Only take into account some labels * Reword docs * fix: crash when updating import.http config (#2204) * fix: crash when updating import.http config * fix key/pattern logic for the attribute processor (#2124) * fix: Update postgres exporter (#2019) * Update postgres exporter * Update changelog * Use postgres exporter branch that implements exporter package * Add TODO for future maintainers * Update VERSION file * Add missing changelog entry * Fix pyroscope.write issues with pyroscope.receive_http (#2201) * Fix pyroscope.write issues with pyroscope.receive_http The nodejs Pyroscope SDK sends profiles with a `Connection: close` header. This header was copied to the upstream request, causing connection churn towards Pyroscope, which can be quite bad on the CPU when using TLS. Do not copy the `Connection` header from the incoming request to fix this issue. Additionally, `pyroscope.write` had a single `http.Client` used for forwarding data from `pyroscope.receive_http`, which may not work if multiple endpoints are configured with different options. To fix this, store a `http.Client` for each endpoint. --------- Co-authored-by: YusifAghalar <[email protected]> Co-authored-by: Piotr <[email protected]> Co-authored-by: Sam DeHaan <[email protected]> Co-authored-by: Craig Peterson <[email protected]> Co-authored-by: Marc Sanmiquel <[email protected]> Co-authored-by: Sergei Nikolaev <[email protected]> Co-authored-by: William Dumont <[email protected]> Co-authored-by: Sam DeHaan <[email protected]> Co-authored-by: Gergely Madarász <[email protected]>

fix: crash when updating import.http config

08e11e8

kinolaev requested a review from a team as a code owner December 2, 2024 18:41

ptodev approved these changes Dec 2, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

chore: update changelog

d2dbcab

Co-authored-by: Paulin Todev <[email protected]>

ptodev merged commit 9177f33 into grafana:main Dec 3, 2024
14 of 15 checks passed

ptodev pushed a commit that referenced this pull request Dec 3, 2024

fix: crash when updating import.http config (#2204)

5d10f6d

* fix: crash when updating import.http config

github-actions bot added the frozen-due-to-age label Jan 3, 2025

github-actions bot locked as resolved and limited conversation to collaborators Jan 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: crash when updating import.http config #2204

fix: crash when updating import.http config #2204

kinolaev commented Dec 2, 2024

CLAassistant commented Dec 2, 2024 •

edited

Loading

kinolaev commented Dec 2, 2024

ptodev commented Dec 2, 2024

kinolaev commented Dec 3, 2024

fix: crash when updating import.http config #2204

fix: crash when updating import.http config #2204

Conversation

kinolaev commented Dec 2, 2024

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

CLAassistant commented Dec 2, 2024 • edited Loading

kinolaev commented Dec 2, 2024

ptodev commented Dec 2, 2024

kinolaev commented Dec 3, 2024

CLAassistant commented Dec 2, 2024 •

edited

Loading