Restore --prefix CLI argument #5
Conversation
Co-authored-by: xDarksome <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR restores the documented --prefix CLI argument that was missing from the implementation. The --prefix flag allows users to set a directory prefix for parquet files via the command line, complementing the existing label-based approach (prom2parquet_prefix label).
Key Changes
- Added
prefixfield to theoptionsstruct to store the CLI argument value - Registered the
--prefixflag in the root command with an empty string default - Updated prefix resolution logic to prioritize label-based prefix over CLI prefix, with fallback to no prefix
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmd/args.go | Added prefix field to options struct to store CLI argument |
| cmd/root.go | Registered --prefix flag with appropriate help text and default value |
| cmd/server.go | Updated sendTimeseries to use label prefix first, then CLI prefix, then no prefix |
| cmd/server_test.go | Added tests for CLI prefix usage and no-prefix scenarios |
| .gitignore | Added build directory to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestSendTimeseriesWithCLIPrefix(t *testing.T) { | ||
| cliPrefix := "cli-prefix" | ||
| srv := newServer(&options{prefix: cliPrefix}) | ||
| expectedChannelName := cliPrefix + "/" + metricName | ||
| srv.channels[expectedChannelName] = make(chan prompb.TimeSeries) | ||
|
|
||
| ts := prompb.TimeSeries{ | ||
| Labels: []prompb.Label{ | ||
| { | ||
| Name: model.MetricNameLabel, | ||
| Value: metricName, | ||
| }, | ||
| { | ||
| Name: "foo", | ||
| Value: "bar", | ||
| }, | ||
| }, | ||
| Samples: []prompb.Sample{ | ||
| { | ||
| Value: 1.0, | ||
| Timestamp: 0, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| go func() { | ||
| err := srv.sendTimeseries(context.TODO(), []prompb.TimeSeries{ts}) | ||
| assert.Nil(t, err) | ||
| }() | ||
|
|
||
| val := <-srv.channels[expectedChannelName] | ||
| assert.Equal(t, ts, val) | ||
| } | ||
|
|
||
| func TestSendTimeseriesWithoutPrefix(t *testing.T) { | ||
| srv := newServer(&options{}) | ||
| srv.channels[metricName] = make(chan prompb.TimeSeries) | ||
|
|
||
| ts := prompb.TimeSeries{ | ||
| Labels: []prompb.Label{ | ||
| { | ||
| Name: model.MetricNameLabel, | ||
| Value: metricName, | ||
| }, | ||
| { | ||
| Name: "foo", | ||
| Value: "bar", | ||
| }, | ||
| }, | ||
| Samples: []prompb.Sample{ | ||
| { | ||
| Value: 1.0, | ||
| Timestamp: 0, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| go func() { | ||
| err := srv.sendTimeseries(context.TODO(), []prompb.TimeSeries{ts}) | ||
| assert.Nil(t, err) | ||
| }() | ||
|
|
||
| val := <-srv.channels[metricName] | ||
| assert.Equal(t, ts, val) | ||
| } |
There was a problem hiding this comment.
Missing test case for label priority: The implementation correctly prioritizes the prom2parquet_prefix label over the CLI --prefix argument (lines 122-125 in server.go), but there is no test verifying this behavior. Consider adding a test case that provides both a CLI prefix and a label prefix to ensure the label takes precedence as documented.
The README documented a
--prefixargument for setting the directory prefix for parquet files, but the flag was removed from the CLI implementation.Changes
prefixfield tooptionsstruct--prefixflag in rootCmd with empty string defaultsendTimeseries:prom2parquet_prefixlabel if present--prefixif label absent/emptyBehavior
The implementation maintains backward compatibility for existing deployments using the label-based approach while restoring the documented CLI functionality.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.