Apply go fix ./...#8074
Merged
Merged
Conversation
Signed-off-by: Yuri Shkuro <github@ysh.us>
yurishkuro
added
the
changelog:refactoring
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request applies automated Go tooling fixes ("go fix") to modernize the codebase with Go 1.23+ features. However, the automated changes contain critical bugs that will prevent compilation, making this PR unsuitable for merging in its current state.
Changes:
- Modernizes loop syntax using
for range ninstead offor i := 0; i < n; i++ - Updates string manipulation to use
strings.CutPrefixinstead ofHasPrefix+TrimPrefix - Adopts
maps.Copyfor map copying andreflect.TypeForfor type reflection - Uses
sync.WaitGroup.Go()method for goroutine management - Attempts to replace pointer creation pattern
&vwithnew(v)(this is incorrect) - Updates other patterns including import aliases and formatting functions
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/storage/v2/memory/tenant.go | Updates string prefix checking to use strings.CutPrefix |
| internal/storage/v2/memory/sampling_test.go | Simplifies loop syntax to for range n |
| internal/storage/v2/elasticsearch/tracestore/writer.go | Simplifies loop to for i range slice |
| internal/storage/v2/elasticsearch/tracestore/from_dbmodel_test.go | Simplifies loop syntax |
| internal/storage/v2/clickhouse/tracestore/query_builder.go | Simplifies loop for indentation |
| internal/storage/v2/cassandra/tracestore/from_dbmodel_test.go | Simplifies loop syntax |
| internal/storage/v1/elasticsearch/spanstore/reader_test.go | Simplifies loop syntax |
| internal/storage/v1/elasticsearch/options.go | CRITICAL BUG: Incorrectly changes &v to new(v) |
| internal/storage/v1/elasticsearch/mappings/mapping_test.go | CRITICAL BUG: Incorrectly uses new(value) instead of ptr(value) |
| internal/storage/v1/elasticsearch/mappings/command_test.go | CRITICAL BUG: Incorrectly uses new(value) instead of ptr(value) |
| internal/storage/v1/elasticsearch/factory_test.go | CRITICAL BUG: Incorrectly uses new(value) instead of ptr(value) |
| internal/storage/v1/badger/spanstore/rw_internal_test.go | Simplifies loop syntax |
| internal/storage/v1/badger/spanstore/reader.go | Simplifies loop syntax |
| internal/storage/v1/badger/spanstore/read_write_test.go | Simplifies loop syntax |
| internal/storage/v1/badger/factory_test.go | Simplifies loop syntax |
| internal/storage/v1/badger/dependencystore/storage_test.go | Simplifies loop syntax |
| internal/storage/metricstore/elasticsearch/processor_test.go | CRITICAL BUG: Incorrectly changes &d to new(d) |
| internal/storage/integration/integration.go | Simplifies loop syntax |
| internal/storage/integration/elasticsearch_test.go | Simplifies loop syntax |
| internal/storage/elasticsearch/dbmodel/model.go | CRITICAL: Removes omitempty JSON tag (breaking change) |
| internal/storage/elasticsearch/config/config_test.go | CRITICAL BUG: Incorrectly changes &v to new(v) |
| internal/storage/elasticsearch/config/config.go | Updates map copying to use maps.Copy |
| internal/sampling/samplingstrategy/file/provider_test.go | Simplifies loop syntax |
| internal/sampling/samplingstrategy/adaptive/weightvectorcache.go | Simplifies loop syntax |
| internal/sampling/samplingstrategy/adaptive/provider_test.go | Simplifies loop syntax |
| internal/sampling/samplingstrategy/adaptive/provider.go | Uses WaitGroup.Go() method |
| internal/sampling/samplingstrategy/adaptive/post_aggregator_test.go | Simplifies loop syntax |
| internal/sampling/samplingstrategy/adaptive/post_aggregator.go | Simplifies loop syntax |
| internal/sampling/samplingstrategy/adaptive/aggregator_test.go | Simplifies loop syntax |
| internal/sampling/samplingstrategy/adaptive/aggregator.go | Uses WaitGroup.Go() method |
| internal/sampling/grpc/grpc_handler_test.go | Renames context import to context0 (unconventional) |
| internal/metrics/metrics_test.go | Simplifies loop syntax |
| internal/metrics/metrics.go | Updates to use reflect.TypeFor |
| internal/leaderelection/leader_election_test.go | Simplifies loop syntax |
| internal/gogocodec/codec_test.go | Updates to use reflect.TypeFor and simplifies loop |
| internal/cache/lru_test.go | Simplifies loop syntax |
| internal/auth/tokenloader_test.go | Simplifies loop syntax |
| examples/hotrod/services/driver/server.go | BUG: Variable shadowing in nested loops |
| examples/hotrod/pkg/pool/pool.go | Simplifies loop syntax |
| cmd/remote-storage/app/server.go | Uses WaitGroup.Go() method |
| cmd/jaeger/internal/extension/remotesampling/extension.go | Uses WaitGroup.Go() method |
| cmd/jaeger/internal/extension/jaegerquery/internal/static_handler_test.go | Simplifies loop syntax |
| cmd/jaeger/internal/extension/jaegerquery/internal/static_handler.go | Changes fmt.Sprintf to fmt.Appendf |
| cmd/jaeger/internal/extension/jaegerquery/internal/server.go | Uses WaitGroup.Go() method |
| cmd/jaeger/internal/extension/jaegerquery/internal/http_handler_test.go | Simplifies loop syntax |
| cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_span_names_test.go | Simplifies loop syntax |
| cmd/jaeger/internal/extension/jaegermcp/internal/handlers/get_services_test.go | Simplifies loop syntax |
| cmd/jaeger/internal/extension/jaegermcp/internal/criticalpath/sanitize_test.go | Updates to use maps.Copy |
| cmd/jaeger/internal/extension/jaegermcp/internal/criticalpath/get_child_of_spans_test.go | Updates to use maps.Copy |
| cmd/jaeger/internal/extension/expvar/extension.go | Uses WaitGroup.Go() method |
| cmd/internal/storageconfig/config_test.go | Updates to use reflect.TypeFor and Type.Fields() |
| cmd/internal/flags/service_test.go | Simplifies loop syntax |
| cmd/es-rollover/app/init/flags.go | CRITICAL BUG: Incorrectly changes &v to new(v) |
| cmd/es-rollover/app/init/action_test.go | CRITICAL BUG: Incorrectly uses new(value) |
| cmd/anonymizer/app/writer/writer_test.go | Simplifies loop syntax |
| cmd/anonymizer/app/query/query_test.go | Uses WaitGroup.Go() method |
| cmd/anonymizer/app/anonymizer/anonymizer.go | Uses WaitGroup.Go() method |
Comments suppressed due to low confidence (1)
examples/hotrod/services/driver/server.go:78
- The inner loop variable
ishadows the outer loop variablei(from line 70). This creates a bug because line 78 usesi+1expecting the retry counter, butinow refers to the inner loop variable which ranges from 0-2. The error log message will incorrectly show retry numbers 1, 2, 3 instead of the intended values. The inner loop should use a different variable name, such asretryCountorattempt.
for i := range 3 {
drv, err = s.redis.GetDriver(ctx, driverID)
if err == nil {
break
}
s.logger.For(ctx).Error("Retrying GetDriver after error", zap.Int("retry_no", i+1), zap.Error(err))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
jaegertracingbot
approved these changes
Feb 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 58 changed files in this pull request and generated 18 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| EsVersion: 7, | ||
| Shards: 5, | ||
| Replicas: ptr(int64(1)), | ||
| Replicas: new(int64(1)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(1)) is syntactically incorrect.
Comment on lines
+744
to
+754
| Replicas: new(int64(1)), | ||
| Priority: 10, | ||
| }, | ||
| Services: IndexOptions{ | ||
| Shards: 5, | ||
| Replicas: int64Ptr(1), | ||
| Replicas: new(int64(1)), | ||
| Priority: 20, | ||
| }, | ||
| Dependencies: IndexOptions{ | ||
| Shards: 5, | ||
| Replicas: int64Ptr(1), | ||
| Replicas: new(int64(1)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. All instances of new(int64(1)) in this struct are syntactically incorrect.
| Services: escfg.IndexOptions{ | ||
| Shards: 3, | ||
| Replicas: ptr(int64(1)), | ||
| Replicas: new(int64(1)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(1)) is syntactically incorrect.
| return config.IndexOptions{ | ||
| Shards: 3, | ||
| Replicas: ptr(int64(3)), | ||
| Replicas: new(int64(3)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(3)) is syntactically incorrect. The correct approach would be to use a helper function or inline pointer creation.
| Spans: config.IndexOptions{ | ||
| Shards: 2, | ||
| Replicas: ptr(int64(1)), | ||
| Replicas: new(int64(1)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(1)) is syntactically incorrect.
| indexTemOps := config.IndexOptions{ | ||
| Shards: 3, | ||
| Replicas: ptr(int64(5)), | ||
| Replicas: new(int64(5)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(5)) is syntactically incorrect.
| indexTemOps := config.IndexOptions{ | ||
| Shards: 3, | ||
| Replicas: ptr(int64(3)), | ||
| Replicas: new(int64(3)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(3)) is syntactically incorrect.
| Indices: config.Indices{ | ||
| Dependencies: config.IndexOptions{ | ||
| Replicas: ptr(int64(1)), | ||
| Replicas: new(int64(1)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(1)) is syntactically incorrect.
Comment on lines
+88
to
+100
| name: "ES version 7", args: Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: new(int64(1)), IndexPrefix: "test", UseILM: "true", ILMPolicyName: "jaeger-test-policy"}, | ||
| want: "ES version 7", | ||
| }, | ||
| { | ||
| name: "Parse Error version 7", args: Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: ptr(int64(1)), IndexPrefix: "test", UseILM: "true", ILMPolicyName: "jaeger-test-policy"}, | ||
| name: "Parse Error version 7", args: Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: new(int64(1)), IndexPrefix: "test", UseILM: "true", ILMPolicyName: "jaeger-test-policy"}, | ||
| wantErr: errors.New("parse error"), | ||
| }, | ||
| { | ||
| name: "Parse bool error", args: Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: ptr(int64(1)), IndexPrefix: "test", UseILM: "foo", ILMPolicyName: "jaeger-test-policy"}, | ||
| name: "Parse bool error", args: Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: new(int64(1)), IndexPrefix: "test", UseILM: "foo", ILMPolicyName: "jaeger-test-policy"}, | ||
| wantErr: errors.New("strconv.ParseBool: parsing \"foo\": invalid syntax"), | ||
| }, | ||
| { | ||
| name: "Invalid Mapping type", args: Options{Mapping: "invalid-mapping", EsVersion: 7, Shards: 5, Replicas: ptr(int64(1)), IndexPrefix: "test", UseILM: "true", ILMPolicyName: "jaeger-test-policy"}, | ||
| name: "Invalid Mapping type", args: Options{Mapping: "invalid-mapping", EsVersion: 7, Shards: 5, Replicas: new(int64(1)), IndexPrefix: "test", UseILM: "true", ILMPolicyName: "jaeger-test-policy"}, |
There was a problem hiding this comment.
Invalid use of new() builtin in struct literal. The new() function takes a type as argument, not a value. All instances of new(int64(1)) are syntactically incorrect.
| EsVersion: 7, | ||
| Shards: 5, | ||
| Replicas: ptr(int64(1)), | ||
| Replicas: new(int64(1)), |
There was a problem hiding this comment.
Invalid use of new() builtin. The new() function takes a type as argument, not a value. new(int64(1)) is syntactically incorrect.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8074 +/- ##
==========================================
+ Coverage 95.47% 95.49% +0.02%
==========================================
Files 316 316
Lines 16756 16732 -24
==========================================
- Hits 15997 15979 -18
+ Misses 593 588 -5
+ Partials 166 165 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Apply
go fix ./...modernizations with some manual clean-up afterwards.