Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NPM-3749] Fix revive warnings from golangci-ling 1.60.3 upgrade #33039

Merged
merged 9 commits into from
Jan 24, 2025
23 changes: 11 additions & 12 deletions pkg/dynamicinstrumentation/codegen/codegen.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,17 @@ func generateHeaderText(param *ditypes.Parameter, out io.Writer) error {
return generateSliceHeader(param, out)
} else if reflect.Kind(param.Kind) == reflect.String {
return generateStringHeader(param, out)
} else { //nolint:revive // TODO
tmplt, err := resolveHeaderTemplate(param)
if err != nil {
return err
}
err = tmplt.Execute(out, param)
if err != nil {
return err
}
if len(param.ParameterPieces) != 0 {
return generateHeadersText(param.ParameterPieces, out)
}
}
template, err := resolveHeaderTemplate(param)
if err != nil {
return err
}
err = template.Execute(out, param)
if err != nil {
return err
}
if len(param.ParameterPieces) != 0 {
return generateHeadersText(param.ParameterPieces, out)
}
return nil
}
Expand Down
29 changes: 4 additions & 25 deletions pkg/dynamicinstrumentation/di.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
package dynamicinstrumentation

import (
"encoding/json"
"fmt"
"io"

Expand Down Expand Up @@ -51,14 +50,16 @@ func newGoDIStats() GoDIStats {
}
}

type OfflineOptions struct { //nolint:revive // TODO
// OfflineOptions configures the Offline options for the running Dynamic Instrumentation process
type OfflineOptions struct {
Offline bool
ProbesFilePath string
SnapshotOutput string
DiagnosticOutput string
}

type ReaderWriterOptions struct { //nolint:revive // TODO
// ReaderWriterOptions configures the ReaderWriter options for the running Dynamic Instrumentation process
type ReaderWriterOptions struct {
CustomReaderWriters bool
SnapshotWriter io.Writer
DiagnosticWriter io.Writer
Expand Down Expand Up @@ -153,28 +154,6 @@ func RunDynamicInstrumentation(opts *DIOptions) (*GoDI, error) {
return goDI, nil
}

func (goDI *GoDI) printSnapshot(event *ditypes.DIEvent) { //nolint:unused // TODO
if event == nil {
return
}
procInfo := goDI.ConfigManager.GetProcInfos()[event.PID]
diLog := uploader.NewDILog(procInfo, event)

var bs []byte
var err error

if diLog != nil {
bs, err = json.MarshalIndent(diLog, "", " ")
} else {
bs, err = json.MarshalIndent(event, "", " ")
}

if err != nil {
log.Info(err)
}
log.Debug(string(bs))
}

func (goDI *GoDI) uploadSnapshot(event *ditypes.DIEvent) {
// goDI.printSnapshot(event)
procInfo := goDI.ConfigManager.GetProcInfos()[event.PID]
Expand Down
3 changes: 2 additions & 1 deletion pkg/dynamicinstrumentation/diagnostics/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ func (m *DiagnosticManager) update(id probeInstanceID, d *ditypes.DiagnosticUplo
}
}

func StopGlobalDiagnostics() { //nolint:revive // TODO
// StopGlobalDiagnostics stops diagnostics from running and closes the updates channel
func StopGlobalDiagnostics() {
close(Diagnostics.Updates)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/dynamicinstrumentation/diconfig/config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (cm *RCConfigManager) installConfigProbe(procInfo *ditypes.ProcessInfo) err
return fmt.Errorf("could not generate bpf code for config probe: %w", err)
}

err = ebpf.CompileBPFProgram(procInfo, configProbe)
err = ebpf.CompileBPFProgram(configProbe)
if err != nil {
return fmt.Errorf("could not compile bpf code for config probe: %w", err)
}
Expand Down Expand Up @@ -262,7 +262,7 @@ generateCompileAttach:
return
}

err = ebpf.CompileBPFProgram(procInfo, probe)
err = ebpf.CompileBPFProgram(probe)
if err != nil {
// TODO: Emit diagnostic?
log.Info("Couldn't compile BPF object", err)
Expand Down
8 changes: 6 additions & 2 deletions pkg/dynamicinstrumentation/diconfig/file_config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import (
"github.com/DataDog/datadog-agent/pkg/util/log"
)

func NewFileConfigManager(configFile string) (*ReaderConfigManager, func(), error) { //nolint:revive // TODO
// NewFileConfigManager creates a new FileConfigManager
func NewFileConfigManager(configFile string) (*ReaderConfigManager, func(), error) {
stopChan := make(chan bool)
stop := func() {
stopChan <- true
Expand All @@ -35,7 +36,10 @@ func NewFileConfigManager(configFile string) (*ReaderConfigManager, func(), erro
for {
select {
case rawBytes := <-updateChan:
cm.ConfigWriter.Write(rawBytes) //nolint:errcheck // TODO
_, err := cm.ConfigWriter.Write(rawBytes)
if err != nil {
log.Errorf("Error writing config file %s: %s", configFile, err)
}
case <-stopChan:
log.Info("stopping file config manager")
fw.Stop()
Expand Down
33 changes: 20 additions & 13 deletions pkg/dynamicinstrumentation/diconfig/mem_config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ type ReaderConfigManager struct {
state ditypes.DIProcs
}

type readerConfigCallback func(configsByService) //nolint:unused // TODO
type configsByService = map[ditypes.ServiceName]map[ditypes.ProbeID]rcConfig

func NewReaderConfigManager() (*ReaderConfigManager, error) { //nolint:revive // TODO
// NewReaderConfigManager creates a new ReaderConfigManager
func NewReaderConfigManager() (*ReaderConfigManager, error) {
cm := &ReaderConfigManager{
callback: applyConfigUpdate,
state: ditypes.NewDIProcs(),
Expand All @@ -55,11 +55,13 @@ func NewReaderConfigManager() (*ReaderConfigManager, error) { //nolint:revive //
return cm, nil
}

func (cm *ReaderConfigManager) GetProcInfos() ditypes.DIProcs { //nolint:revive // TODO
// GetProcInfos returns the process info state
func (cm *ReaderConfigManager) GetProcInfos() ditypes.DIProcs {
return cm.state
}

func (cm *ReaderConfigManager) Stop() { //nolint:revive // TODO
// Stop causes the ReaderConfigManager to stop processing data
func (cm *ReaderConfigManager) Stop() {
cm.ConfigWriter.Stop()
cm.procTracker.Stop()
}
Expand Down Expand Up @@ -131,17 +133,20 @@ func (cm *ReaderConfigManager) updateServiceConfigs(configs configsByService) {
}
}

type ConfigWriter struct { //nolint:revive // TODO
// ConfigWriter handles writing configuration data
type ConfigWriter struct {
io.Writer
updateChannel chan ([]byte)
Processes map[ditypes.PID]*ditypes.ProcessInfo
configCallback ConfigWriterCallback
stopChannel chan (bool)
}

type ConfigWriterCallback func(configsByService) //nolint:revive // TODO
// ConfigWriterCallback provides a callback interface for ConfigWriter
type ConfigWriterCallback func(configsByService)

func NewConfigWriter(onConfigUpdate ConfigWriterCallback) *ConfigWriter { //nolint:revive // TODO
// NewConfigWriter creates a new ConfigWriter
func NewConfigWriter(onConfigUpdate ConfigWriterCallback) *ConfigWriter {
return &ConfigWriter{
updateChannel: make(chan []byte, 1),
configCallback: onConfigUpdate,
Expand All @@ -154,7 +159,8 @@ func (r *ConfigWriter) Write(p []byte) (n int, e error) {
return 0, nil
}

func (r *ConfigWriter) Start() error { //nolint:revive // TODO
// Start initiates the ConfigWriter to start processing data
func (r *ConfigWriter) Start() error {
go func() {
configUpdateLoop:
for {
Expand All @@ -175,18 +181,19 @@ func (r *ConfigWriter) Start() error { //nolint:revive // TODO
return nil
}

func (cu *ConfigWriter) Stop() { //nolint:revive // TODO
cu.stopChannel <- true
// Stop causes the ConfigWriter to stop processing data
func (r *ConfigWriter) Stop() {
r.stopChannel <- true
}

// UpdateProcesses is the callback interface that ConfigWriter uses to consume the map of ProcessInfo's
// such that it's used whenever there's an update to the state of known service processes on the machine.
// It simply overwrites the previous state of known service processes with the new one
func (cu *ConfigWriter) UpdateProcesses(procs ditypes.DIProcs) { //nolint:revive // TODO
func (r *ConfigWriter) UpdateProcesses(procs ditypes.DIProcs) {
current := procs
old := cu.Processes
old := r.Processes
if !reflect.DeepEqual(current, old) {
cu.Processes = current
r.Processes = current
}
}

Expand Down
9 changes: 2 additions & 7 deletions pkg/dynamicinstrumentation/ebpf/ebpf.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"fmt"
"io"
matthewleese marked this conversation as resolved.
Show resolved Hide resolved
"text/template"
"time"

"github.com/cilium/ebpf"
"github.com/cilium/ebpf/link"
Expand Down Expand Up @@ -126,8 +125,8 @@ func AttachBPFUprobe(procInfo *ditypes.ProcessInfo, probe *ditypes.Probe) error
return nil
}

// CompileBPFProgram compiles the code for a single probe associated with the process given by procInfo
func CompileBPFProgram(procInfo *ditypes.ProcessInfo, probe *ditypes.Probe) error { //nolint:revive // TODO
// CompileBPFProgram compiles the code for a single probe
func CompileBPFProgram(probe *ditypes.Probe) error {
f := func(in io.Reader, out io.Writer) error {
fileContents, err := io.ReadAll(in)
if err != nil {
Expand Down Expand Up @@ -169,7 +168,3 @@ func getCFlags(config *ddebpf.Config) []string {
}
return cflags
}

const (
compilationStepTimeout = 60 * time.Second //nolint:unused // TODO
)
4 changes: 2 additions & 2 deletions pkg/dynamicinstrumentation/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type Module struct {
}

// NewModule creates a new dynamic instrumentation system probe module
func NewModule(config *Config) (*Module, error) { //nolint:revive // TODO
func NewModule(_ *Config) (*Module, error) {
godi, err := di.RunDynamicInstrumentation(&di.DIOptions{
RateLimitPerProbePerSecond: 1.0,
OfflineOptions: di.OfflineOptions{
Expand Down Expand Up @@ -66,7 +66,7 @@ func (m *Module) GetStats() map[string]interface{} {
// Register creates a health check endpoint for the dynamic instrumentation module
func (m *Module) Register(httpMux *module.Router) error {
httpMux.HandleFunc("/check", utils.WithConcurrencyLimit(utils.DefaultMaxConcurrentRequests,
func(w http.ResponseWriter, req *http.Request) { //nolint:revive // TODO
func(w http.ResponseWriter, _ *http.Request) {
stats := []string{}
utils.WriteAsJSON(w, stats)
}))
Expand Down
74 changes: 0 additions & 74 deletions pkg/dynamicinstrumentation/testutil/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,54 +40,6 @@ var basicCaptures = fixtures{
// "github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.test_single_float64": {"x": capturedValue("float", "-1.646464")},
}

var multiParamCaptures = fixtures{ //nolint:unused // TODO
"github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.test_multiple_simple_params": {
"a": capturedValue("bool", "false"),
"b": capturedValue("uint8", "42"),
"c": capturedValue("int32", "122"),
"d": capturedValue("uint", "1337"),
"e": capturedValue("string", "xyz"),
},
"github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.test_multiple_composite_params": {
"a": {Type: "array", Fields: fieldMap{
"a_0": capturedValue("string", "one"),
"a_1": capturedValue("string", "two"),
"a_2": capturedValue("string", "three"),
}},
"b": {Type: "struct", Fields: fieldMap{
"aBool": capturedValue("bool", "false"),
"aString": capturedValue("string", ""),
"aNumber": capturedValue("int", "0"),
"nested": {Type: "struct", Fields: fieldMap{
"anotherInt": capturedValue("int", "0"),
"anotherString": capturedValue("string", ""),
}},
}},
"c": {Type: "slice", Fields: fieldMap{
"c_0": capturedValue("uint", "24"),
"c_1": capturedValue("uint", "42"),
}},
"d": {Type: "map", Fields: fieldMap{
"foo": capturedValue("string", "bar"),
}},
"e": {Type: "slice", Fields: fieldMap{
"e_0": {Type: "struct", Fields: fieldMap{
"anotherInt": capturedValue("int", "42"),
"anotherString": capturedValue("string", "ftwo"),
}},
"e_1": {Type: "struct", Fields: fieldMap{
"anotherInt": capturedValue("int", "24"),
"anotherString": capturedValue("string", "tfour"),
}},
}},
},
"github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.test_combined_byte": {
"w": capturedValue("uint8", "2"),
"x": capturedValue("uint8", "3"),
"y": capturedValue("uint8", "3.0"),
},
}

var stringCaptures = fixtures{
"github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.test_single_string": {"x": capturedValue("string", "abc")},
}
Expand Down Expand Up @@ -239,32 +191,6 @@ var structCaptures = fixtures{
}}},
}

// TODO: this doesn't work yet:
// could not determine locations of variables from debug information could not inspect param "x" on function: no location field in parameter entry
var genericCaptures = fixtures{ //nolint:unused // TODO
"github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.typeWithGenerics[go.shape.string].Guess": {"value": capturedValue("string", "generics work")},
}

// TODO: check how map entries should be represented, likely that entries have key / value pair fields
// instead of having the keys hardcoded as string field names
// maps are no supported at the moment so this fails anyway
var mapCaptures = fixtures{ //nolint:unused // TODO
"github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.test_map_string_to_int": {"m": {Type: "map", Fields: fieldMap{
"foo": capturedValue("int", "1"),
"bar": capturedValue("int", "2"),
}}},
"github.com/DataDog/datadog-agent/pkg/dynamicinstrumentation/testutil/sample.test_map_string_to_struct": {"m": {Type: "map", Fields: fieldMap{
"foo": {Type: "struct", Fields: fieldMap{
"anotherInt": capturedValue("int", "3"),
"anotherString": capturedValue("string", "four"),
}},
"bar": {Type: "struct", Fields: fieldMap{
"anotherInt": capturedValue("int", "3"),
"anotherString": capturedValue("string", "four"),
}},
}}},
}

// mergeMaps combines multiple fixture maps into a single map
func mergeMaps(maps ...fixtures) fixtures {
result := make(fixtures)
Expand Down
Loading
Loading