Skip to content

Conversation

pipiland2612
Copy link
Contributor

Changes

  • Add MessageFilter to remote_api.go to support for filtering message while call Write

Signed-off-by: pipiland2612 <[email protected]>
@pipiland2612
Copy link
Contributor Author

The test failed seems unrelated to my changes:

Run make check_license test
>> checking license header
make common-deps
make[1]: Entering directory '/home/runner/work/client_golang/client_golang'
>> getting dependencies
go mod download
make[1]: Leaving directory '/home/runner/work/client_golang/client_golang'
cd exp && go mod tidy && go mod download
go: downloading github.com/stretchr/testify v1.11.1
go: downloading gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
go: downloading github.com/rogpeppe/go-internal v1.10.0
go: downloading github.com/kr/text v0.2.0
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading gopkg.in/yaml.v3 v3.0.1
go: downloading github.com/pmezard/go-difflib v1.0.0
>> running all tests
go test -race  ./...
ok  	github.com/prometheus/client_golang/api	1.061s
ok  	github.com/prometheus/client_golang/api/prometheus/v1	1.041s
?   	github.com/prometheus/client_golang/examples/createdtimestamps	[no test files]
?   	github.com/prometheus/client_golang/examples/customlabels	[no test files]
?   	github.com/prometheus/client_golang/examples/exemplars	[no test files]
?   	github.com/prometheus/client_golang/examples/gocollector	[no test files]
?   	github.com/prometheus/client_golang/examples/middleware	[no test files]
?   	github.com/prometheus/client_golang/examples/middleware/httpmiddleware	[no test files]
?   	github.com/prometheus/client_golang/examples/random	[no test files]
?   	github.com/prometheus/client_golang/examples/simple	[no test files]
?   	github.com/prometheus/client_golang/examples/versioncollector	[no test files]
ok  	github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil	1.008s
ok  	github.com/prometheus/client_golang/internal/github.com/golang/gddo/httputil/header	1.014s
--- FAIL: TestGoCollector_ExposedMetrics (0.00s)
    --- FAIL: TestGoCollector_ExposedMetrics/#02 (0.00s)
        go_collector_latest_test.go:131: found unexpected metric go_godebug_non_default_behavior_httpcookiemaxnum_events_total
--- FAIL: TestExpectedRuntimeMetrics (0.00s)
    go_collector_latest_test.go:331: found new runtime/metrics metric /godebug/non-default-behavior/httpcookiemaxnum:events
    go_collector_latest_test.go:383: a new Go version may have been detected, please run
    go_collector_latest_test.go:384: 	go run gen_go_collector_metrics_set.go go1.X
    go_collector_latest_test.go:385: where X is the Go version you are currently using
FAIL
FAIL	github.com/prometheus/client_golang/prometheus	146.077s
--- FAIL: TestGoCollectorAllowList (0.02s)
    --- FAIL: TestGoCollectorAllowList/allow_all (0.01s)
        go_collector_latest_test.go:193: missmatch (-want +got):
              []string{
              	... // 45 identical elements
              	"go_godebug_non_default_behavior_http2client_events_total",
              	"go_godebug_non_default_behavior_http2server_events_total",
            - 	"go_godebug_non_default_behavior_httpcookiemaxnum_events_total",
              	"go_godebug_non_default_behavior_httplaxcontentlength_events_total",
              	"go_godebug_non_default_behavior_httpmuxgo121_events_total",
              	... // 53 identical elements
              }
    --- FAIL: TestGoCollectorAllowList/allow_debug (0.01s)
        go_collector_latest_test.go:193: missmatch (-want +got):
              []string{
              	... // 13 identical elements
              	"go_godebug_non_default_behavior_http2client_events_total",
              	"go_godebug_non_default_behavior_http2server_events_total",
            - 	"go_godebug_non_default_behavior_httpcookiemaxnum_events_total",
              	"go_godebug_non_default_behavior_httplaxcontentlength_events_total",
              	"go_godebug_non_default_behavior_httpmuxgo121_events_total",
              	... // 32 identical elements
              }
FAIL
FAIL	github.com/prometheus/client_golang/prometheus/collectors	0.074s
ok  	github.com/prometheus/client_golang/prometheus/collectors/version	1.023s
ok  	github.com/prometheus/client_golang/prometheus/graphite	1.032s
ok  	github.com/prometheus/client_golang/prometheus/internal	1.019s
ok  	github.com/prometheus/client_golang/prometheus/promauto	1.016s
ok  	github.com/prometheus/client_golang/prometheus/promhttp	1.830s
?   	github.com/prometheus/client_golang/prometheus/promhttp/internal	[no test files]
?   	github.com/prometheus/client_golang/prometheus/promhttp/zstd	[no test files]
ok  	github.com/prometheus/client_golang/prometheus/push	1.034s
ok  	github.com/prometheus/client_golang/prometheus/testutil	1.031s
ok  	github.com/prometheus/client_golang/prometheus/testutil/promlint	1.030s
?   	github.com/prometheus/client_golang/prometheus/testutil/promlint/validations	[no test files]
FAIL
make: *** [Makefile.common:154: common-test] Error 1

@bwplotka
Copy link
Member

bwplotka commented Oct 8, 2025

Hm, something went wrong with #1864 (it was old PR, merged now) -- if you could fix it on the way it would epic, otherwise will fix in the incoming days...

}
})

t.Run("filter can modify message on retries", func(t *testing.T) {
Copy link

@perebaj perebaj Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding, but I'm wondering if the test name fully matches what we're testing here? It seems like we're only attempting once, rather than simulating retry behavior. Does that sound right, or am I missing something?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an idea for more realistic tests: we could implement a custom test server or modify mockStorage to handle retry logic. This would allow us to validate code behavior against scenarios such as 'fails, retries, fails, then succeeds.' Do you feel that this make sense?

@pipiland2612 pipiland2612 mentioned this pull request Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants