Skip to content

Commit ec0584f

Browse files
fix(metricprovider): respect --logformat json for metric plugin process logger
The metric provider plugin client (metricproviders/plugin/client/client.go) constructed its go-plugin ClientConfig without setting Logger, so go-plugin always fell back to its own hardcoded unstructured-text hclog logger for the plugin's handshake/lifecycle logs and stderr relay - regardless of the controller's own --logformat setting. When the controller is run with --logformat json, every other component logs JSON except metric plugin logs, which stay plain text and can't be parsed the same way downstream. Add newPluginLogger(), which mirrors the controller's own logrus formatter: when the standard logger is configured with logrus.JSONFormatter, build an hclog.Logger with JSONFormat: true (keeping go-plugin's other defaults - Trace level, "plugin" name, DefaultOutput - unchanged); otherwise return nil so go-plugin's own default logger is used exactly as before, preserving existing behavior for the common (non-JSON) case. Fixes #4408 Signed-off-by: Sujan Reddy <sujanchalla0510@gmail.com>
1 parent 4e6a279 commit ec0584f

3 files changed

Lines changed: 70 additions & 1 deletion

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ require (
1919
github.com/golang/protobuf v1.5.4
2020
github.com/google/uuid v1.6.0
2121
github.com/grpc-ecosystem/grpc-gateway v1.16.0
22+
github.com/hashicorp/go-hclog v1.6.3
2223
github.com/hashicorp/go-plugin v1.8.0
2324
github.com/influxdata/influxdb-client-go/v2 v2.14.0
2425
github.com/juju/ansiterm v1.0.0
@@ -125,7 +126,6 @@ require (
125126
github.com/gregdel/pushover v1.3.1 // indirect
126127
github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
127128
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
128-
github.com/hashicorp/go-hclog v1.6.3 // indirect
129129
github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
130130
github.com/hashicorp/yamux v0.1.2 // indirect
131131
github.com/huandu/xstrings v1.5.0 // indirect

metricproviders/plugin/client/client.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package client
22

33
import (
44
"fmt"
5+
"io"
56
"os/exec"
67
"sync"
78

9+
"github.com/hashicorp/go-hclog"
810
goPlugin "github.com/hashicorp/go-plugin"
11+
log "github.com/sirupsen/logrus"
912

1013
"github.com/argoproj/argo-rollouts/metricproviders/plugin/rpc"
1114
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
@@ -68,6 +71,7 @@ func (m *metricPlugin) startPluginSystem(metric v1alpha1.Metric) (rpc.MetricProv
6871
Plugins: pluginMap,
6972
Cmd: exec.Command(pluginPath, args...),
7073
Managed: true,
74+
Logger: newPluginLogger(),
7175
})
7276

7377
rpcClient, err := m.pluginClient[pluginName].Client()
@@ -108,3 +112,27 @@ func (m *metricPlugin) startPluginSystem(metric v1alpha1.Metric) (rpc.MetricProv
108112

109113
return nil, fmt.Errorf("no plugin found")
110114
}
115+
116+
// newPluginLogger builds the hclog.Logger used for the go-plugin client's own log output
117+
// (handshake/lifecycle logs, and the plugin process's stderr relay). By default go-plugin
118+
// falls back to its own unstructured text logger whenever ClientConfig.Logger is nil, which
119+
// is inconsistent with the controller's own logs when the controller is run with
120+
// `--logformat json`: every other component's logs are JSON, but metric plugin logs remain
121+
// plain text. When the controller's standard logger is configured for JSON output, mirror
122+
// that here so the plugin logger's output is JSON too. Returns nil (go-plugin's own default
123+
// logger) in every other case, preserving prior behavior exactly.
124+
func newPluginLogger() hclog.Logger {
125+
return newPluginLoggerWithOutput(hclog.DefaultOutput)
126+
}
127+
128+
func newPluginLoggerWithOutput(w io.Writer) hclog.Logger {
129+
if _, ok := log.StandardLogger().Formatter.(*log.JSONFormatter); !ok {
130+
return nil
131+
}
132+
return hclog.New(&hclog.LoggerOptions{
133+
Output: w,
134+
Level: hclog.Trace,
135+
Name: "plugin",
136+
JSONFormat: true,
137+
})
138+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package client
2+
3+
import (
4+
"bytes"
5+
"encoding/json"
6+
"testing"
7+
8+
log "github.com/sirupsen/logrus"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
// TestNewPluginLoggerRespectsControllerLogFormat verifies that the go-plugin client's own
13+
// logger mirrors the controller's configured log format. Before this fix, the plugin
14+
// client's Logger field was always left unset, so go-plugin always fell back to its
15+
// unstructured, hardcoded-text default logger regardless of the controller's own
16+
// `--logformat json` setting.
17+
func TestNewPluginLoggerRespectsControllerLogFormat(t *testing.T) {
18+
origFormatter := log.StandardLogger().Formatter
19+
defer log.StandardLogger().SetFormatter(origFormatter)
20+
21+
t.Run("default text logformat leaves go-plugin's own default logger untouched", func(t *testing.T) {
22+
log.StandardLogger().SetFormatter(&log.TextFormatter{})
23+
logger := newPluginLogger()
24+
assert.Nil(t, logger, "expected nil so go-plugin falls back to its own default logger")
25+
})
26+
27+
t.Run("json logformat produces a JSON plugin logger", func(t *testing.T) {
28+
log.StandardLogger().SetFormatter(&log.JSONFormatter{})
29+
30+
var buf bytes.Buffer
31+
logger := newPluginLoggerWithOutput(&buf)
32+
assert.NotNil(t, logger)
33+
34+
logger.Info("plugin log line", "plugin", "my-plugin")
35+
36+
var parsed map[string]any
37+
err := json.Unmarshal(buf.Bytes(), &parsed)
38+
assert.NoError(t, err, "expected the plugin logger to emit a single JSON object, got: %s", buf.String())
39+
assert.Equal(t, "plugin log line", parsed["@message"])
40+
})
41+
}

0 commit comments

Comments
 (0)