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

chore: Fix linter findings for unparam and revive.unused-parameter #12150

Merged
merged 5 commits into from
Nov 8, 2022
Merged

chore: Fix linter findings for unparam and revive.unused-parameter #12150

merged 5 commits into from
Nov 8, 2022

Conversation

zak-pawel
Copy link
Collaborator

unparam and revive.unused-parameter linters enabled.

Fixes for following findings were made (for default configuration):

agent/tick.go:140:50                                                              unparam      (*UnalignedTicker).start - result 0 (*github.com/influxdata/telegraf/agent.UnalignedTicker) is never used
agent/tick.go:237:48                                                              unparam      (*RollingTicker).start - result 0 (*github.com/influxdata/telegraf/agent.RollingTicker) is never used
cmd/telegraf/main_test.go:30:29                                                   revive       unused-parameter: parameter 'serverErr' seems to be unused, consider removing or renaming it as _
cmd/telegraf/main_test.go:50:46                                                   revive       unused-parameter: parameter 'inFilter' seems to be unused, consider removing or renaming it as _
cmd/telegraf/main_test.go:68:28                                                   revive       unused-parameter: parameter 'address' seems to be unused, consider removing or renaming it as _
config/config.go:1046:82                                                          unparam      (*Config).buildParser - result 1 (error) is always nil
plugins/common/tls/config.go:205:65                                               revive       unused-parameter: parameter 'verifiedChains' seems to be unused, consider removing or renaming it as _
plugins/inputs/amd_rocm_smi/amd_rocm_smi.go:62:45                                 unparam      (*ROCmSMI).pollROCmSMI - result 1 (error) is always nil
plugins/inputs/cloudwatch/cloudwatch.go:320:63                                    unparam      (*CloudWatch).fetchNamespaceMetrics - result 1 (error) is always nil
plugins/inputs/cloudwatch/cloudwatch_test.go:156:76                               revive       unused-parameter: parameter 'params' seems to be unused, consider removing or renaming it as _
plugins/inputs/cloudwatch/cloudwatch_test.go:203:78                               revive       unused-parameter: parameter 'params' seems to be unused, consider removing or renaming it as _
plugins/inputs/cloudwatch_metric_streams/cloudwatch_metric_streams_test.go:72:16  revive       unused-parameter: parameter 'metricStream' seems to be unused, consider removing or renaming it as _
plugins/inputs/cloudwatch_metric_streams/cloudwatch_metric_streams_test.go:72:44  unparam      `createURL` - `rawQuery` always receives `""`
plugins/inputs/procstat/procstat.go:301:121                                       unparam      (*Procstat).updateProcesses - result 0 (error) is always nil
plugins/inputs/sflow/metricencoder.go:11:51                                       unparam      makeMetrics - result 1 (error) is always nil
plugins/outputs/azure_data_explorer/azure_data_explorer_test.go:343:35            revive       unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _
plugins/outputs/azure_data_explorer/azure_data_explorer_test.go:350:33            revive       unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _
plugins/outputs/dynatrace/dynatrace_test.go:519:27                                revive       unused-parameter: parameter 'format' seems to be unused, consider removing or renaming it as _
plugins/outputs/groundwork/groundwork.go:213:98                                   unparam      (*Groundwork).parseMetric - result 2 (error) is always nil
plugins/outputs/postgresql/postgresql.go:277:76                                   unparam      (*Postgresql).writeConcurrent - result 0 (error) is always nil
plugins/outputs/postgresql/table_source_test.go:17:22                             revive       unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _
plugins/parsers/json_v2/parser.go:576:28                                          revive       unused-parameter: parameter 'line' seems to be unused, consider removing or renaming it as _
plugins/parsers/prometheusremotewrite/parser.go:87:33                             revive       unused-parameter: parameter 'config' seems to be unused, consider removing or renaming it as _
plugins/serializers/carbon2/carbon2_test.go:200:24                                unparam      `TestSerializeMetricBool$1` - `t` is unused
tools/custom_builder/config.go:54:70                                              unparam      (*selection).Filter - result 1 (error) is always nil

Moreover, I enabled locally check-exported flag for unparam linter and fixed following findings:

agent/agent.go:27:44                                                          unparam  NewAgent - result 1 (error) is always nil
internal/content_coding.go:138:38                                             unparam  NewGzipEncoder - result 1 (error) is always nil
internal/content_coding.go:166:38                                             unparam  NewZlibEncoder - result 1 (error) is always nil
internal/content_coding.go:212:38                                             unparam  NewGzipDecoder - result 1 (error) is always nil
internal/content_coding.go:243:38                                             unparam  NewZlibDecoder - result 1 (error) is always nil
internal/snmp/translate.go:179:97                                             unparam  `GetIndex` - result `err` is always `nil`
metric/series_grouper.go:45:3                                                 unparam  (*SeriesGrouper).Add - result 0 (error) is always nil
plugins/common/shim/config.go:150:40                                          unparam  DefaultImportedPlugins - result 1 (error) is always nil
plugins/inputs/knx_listener/knx_dummy_interface.go:11:49                      unparam  `NewDummyInterface` - result `err` is always `nil`
plugins/inputs/opcua/read_client.go:146:84                                    unparam  (*ReadClient).StartStreamValues - result 1 (error) is always nil
plugins/inputs/snmp_legacy/snmp_legacy.go:631:12                              unparam  (*Host).HandleResponse - result 1 (error) is always nil
plugins/outputs/postgresql/table_source.go:22:48                              unparam  (*columnList).Add - result 0 (bool) is never used
plugins/parsers/influx/handler.go:41:52                                       unparam  (*MetricHandler).Metric - result 1 (error) is always nil
plugins/serializers/influx/influx.go:127:61                                   unparam  (*Serializer).Write - result 0 (int) is never used
plugins/serializers/prometheus/prometheus.go:49:55                            unparam  NewSerializer - result 1 (error) is always nil
plugins/serializers/prometheusremotewrite/prometheusremotewrite.go:46:55      unparam  NewSerializer - result 1 (error) is always nil
plugins/serializers/registry.go:239:61                                        unparam  NewInfluxSerializerConfig - result 1 (error) is always nil
plugins/serializers/registry.go:257:41                                        unparam  NewInfluxSerializer - result 1 (error) is always nil
plugins/serializers/registry.go:290:42                                        unparam  NewMsgpackSerializer - result 1 (error) is always nil
plugins/serializers/splunkmetric/splunkmetric.go:34:124                       unparam  NewSerializer - result 1 (error) is always nil
plugins/serializers/wavefront/wavefront.go:51:129                             unparam  NewSerializer - result 1 (error) is always nil
testutil/container.go:135:33                                                  unparam  (*Container).Terminate - result 0 (error) is always nil

Unfortunatelly unparam linter is not able to check if function with result ... is always nil problem implements interface (so it must return something), thus following findings weren't fixed and check-exported was not enabled globally:

models/running_processor_test.go:50:38                                        unparam  (*MockProcessorToInit).Init - result 0 (error) is always nil
plugins/aggregators/basicstats/basicstats.go:284:29                           unparam  (*BasicStats).Init - result 0 (error) is always nil
plugins/aggregators/derivative/derivative.go:173:29                           unparam  (*Derivative).Init - result 0 (error) is always nil
plugins/aggregators/merge/merge.go:24:24                                      unparam  (*Merge).Init - result 0 (error) is always nil
plugins/common/starlark/metric.go:74:53                                       unparam  (*Metric).Attr - result 1 (error) is always nil
plugins/inputs/aliyuncms/aliyuncms.go:208:49                                  unparam  (*AliyunCMS).Start - result 0 (error) is always nil
plugins/inputs/bind/bind.go:33:23                                             unparam  (*Bind).Init - result 0 (error) is always nil
plugins/inputs/cassandra/cassandra.go:240:51                                  unparam  (*Cassandra).Start - result 0 (error) is always nil
plugins/inputs/cloudwatch_metric_streams/cloudwatch_metric_streams.go:402:44  unparam  (*CloudWatchMetricStreams).Init - result 0 (error) is always nil
plugins/inputs/consul/consul.go:40:25                                         unparam  (*Consul).Init - result 0 (error) is always nil
plugins/inputs/cpu/cpu.go:137:27                                              unparam  (*CPUStats).Init - result 0 (error) is always nil
plugins/inputs/directory_monitor/directory_monitor.go:138:66                  unparam  (*DirectoryMonitor).Start - result 0 (error) is always nil
plugins/inputs/disk/disk.go:33:29                                             unparam  (*DiskStats).Init - result 0 (error) is always nil
plugins/inputs/http_listener_v2/http_listener_v2.go:89:58                     unparam  (*HTTPListenerV2).Start - result 0 (error) is always nil
plugins/inputs/influxdb_listener/influxdb_listener.go:92:35                   unparam  (*InfluxDBListener).Init - result 0 (error) is always nil
plugins/inputs/influxdb_v2_listener/influxdb_v2_listener.go:103:37            unparam  (*InfluxDBV2Listener).Init - result 0 (error) is always nil
plugins/inputs/internet_speed/internet_speed.go:33:33                         unparam  (*InternetSpeed).Init - result 0 (error) is always nil
plugins/inputs/logparser/logparser.go:89:34                                   unparam  (*LogParserPlugin).Init - result 0 (error) is always nil
plugins/inputs/mailchimp/mailchimp.go:30:28                                   unparam  (*MailChimp).Init - result 0 (error) is always nil
plugins/inputs/mem/mem.go:26:28                                               unparam  (*MemStats).Init - result 0 (error) is always nil
plugins/inputs/mock/mock.go:67:23                                             unparam  (*Mock).Init - result 0 (error) is always nil
plugins/inputs/multifile/multifile.go:37:28                                   unparam  (*MultiFile).Init - result 0 (error) is always nil
plugins/inputs/nfsclient/nfsclient.go:318:28                                  unparam  (*NFSClient).Init - result 0 (error) is always nil
plugins/inputs/postgresql/postgresql.go:38:29                                 unparam  (*Postgresql).Init - result 0 (error) is always nil
plugins/inputs/procstat/procstat.go:514:27                                    unparam  (*Procstat).Init - result 0 (error) is always nil
plugins/inputs/sflow/sflow.go:42:24                                           unparam  (*SFlow).Init - result 0 (error) is always nil
plugins/inputs/sqlserver/sqlserver.go:418:28                                  unparam  (*SQLServer).Init - result 0 (error) is always nil
plugins/outputs/exec/exec.go:41:23                                            unparam  (*Exec).Init - result 0 (error) is always nil
plugins/outputs/prometheus_client/v1/collector.go:192:52                      unparam  (*Collector).Add - result 0 (error) is always nil
plugins/outputs/prometheus_client/v2/collector.go:85:52                       unparam  (*Collector).Add - result 0 (error) is always nil
plugins/outputs/warp10/warp10.go:209:25                                       unparam  (*Warp10).Init - result 0 (error) is always nil
plugins/parsers/collectd/parser_test.go:18:50                                 unparam  (*AuthMap).Password - result 1 (error) is always nil
plugins/parsers/dropwizard/parser.go:241:57                                   unparam  (*Parser).InitFromConfig - result 0 (error) is always nil
plugins/parsers/form_urlencoded/parser.go:102:57                              unparam  (*Parser).InitFromConfig - result 0 (error) is always nil
plugins/parsers/nagios/parser.go:324:57                                       unparam  (*Parser).InitFromConfig - result 0 (error) is always nil
plugins/parsers/prometheus/parser.go:211:57                                   unparam  (*Parser).InitFromConfig - result 0 (error) is always nil
plugins/processors/port_name/port_name.go:197:28                              unparam  (*PortName).Init - result 0 (error) is always nil
plugins/serializers/carbon2/carbon2.go:61:65                                  unparam  (*Serializer).Serialize - result 1 (error) is always nil
plugins/serializers/carbon2/carbon2.go:65:73                                  unparam  (*Serializer).SerializeBatch - result 1 (error) is always nil
plugins/serializers/graphite/graphite.go:50:73                                unparam  (*GraphiteSerializer).Serialize - result 1 (error) is always nil
plugins/serializers/splunkmetric/splunkmetric.go:44:65                        unparam  (*serializer).Serialize - result 1 (error) is always nil
plugins/serializers/splunkmetric/splunkmetric.go:48:73                        unparam  (*serializer).SerializeBatch - result 1 (error) is always nil
plugins/serializers/wavefront/wavefront.go:101:69                             unparam  (*WavefrontSerializer).Serialize - result 1 (error) is always nil
plugins/serializers/wavefront/wavefront.go:110:82                             unparam  (*WavefrontSerializer).SerializeBatch - result 1 (error) is always nil

@telegraf-tiger telegraf-tiger bot added the chore label Nov 3, 2022
@zak-pawel
Copy link
Collaborator Author

@LarsStegman you introduced few days ago, in this PR #11786 in plugins/inputs/opcua/read_client.go function which is not used:

// StartStreamValues does nothing for the read client, as it has to actively fetch values. The channel is closed immediately.
func (o *ReadClient) StartStreamValues(_ context.Context) (<-chan telegraf.Metric, error) {
	c := make(chan telegraf.Metric)
	defer close(c)
	return c, nil
}

Was that a mistake? Are you ok with removing it in this PR?
@reimda @Hipska @powersj @srebhan Do you know something in this topic?

@zak-pawel
Copy link
Collaborator Author

@powersj You introduced in this PR #11672 in plugins/outputs/postgresql/postgresql.go following function:

func (p *Postgresql) writeConcurrent(tableSources map[string]*TableSource) error {
	for _, tableSource := range tableSources {
		select {
		case p.writeChan <- tableSource:
		case <-p.dbContext.Done():
			return nil
		}
	}
	return nil
}

In this PR I removed return values (it doesn't make sense to return always nil). But I wonder if this function works as it should?
I mean it differs from writeSequential(...)...

@zak-pawel zak-pawel added panic issue that results in panics from Telegraf and removed panic issue that results in panics from Telegraf labels Nov 3, 2022
@LarsStegman
Copy link
Contributor

LarsStegman commented Nov 3, 2022

@LarsStegman you introduced few days ago, in this PR #11786 in plugins/inputs/opcua/read_client.go function which is not used:

// StartStreamValues does nothing for the read client, as it has to actively fetch values. The channel is closed immediately.
func (o *ReadClient) StartStreamValues(_ context.Context) (<-chan telegraf.Metric, error) {
	c := make(chan telegraf.Metric)
	defer close(c)
	return c, nil
}

Was that a mistake? Are you ok with removing it in this PR? @reimda @Hipska @powersj @srebhan Do you know something in this topic?

Yea you can remove it. It is a remainder from when the listener and read plug-in were still combined. I either loaded the listener client or read client which both implemented an interface, but that is not needed anymore now that they are split into separate plugins. Thanks!

@powersj
Copy link
Contributor

powersj commented Nov 8, 2022

FYI going to need to deal with conflicts

@powersj powersj added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 8, 2022
# Conflicts:
#	plugins/inputs/cloudwatch/cloudwatch_test.go
#	plugins/inputs/cloudwatch_metric_streams/cloudwatch_metric_streams_test.go
#	plugins/inputs/opcua/opcua_test.go
#	plugins/inputs/opcua_listener/opcua_listener_test.go
@telegraf-tiger
Copy link
Contributor

telegraf-tiger bot commented Nov 8, 2022

@zak-pawel
Copy link
Collaborator Author

@powersj Deal done!

@powersj powersj merged commit 6816aef into influxdata:master Nov 8, 2022
@@ -100,11 +100,7 @@ func (p *Parser) Parse(input []byte) ([]telegraf.Metric, error) {
}
}

metric, err := p.handler.Metric()
Copy link

Choose a reason for hiding this comment

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

@powersj I was trying to contrast this change with the Graphite Parser and the socketlistener (which uses the parsers).
The Graphite parser continues to return both errors and metrics. However, the socket_listener ignores the metrics if there are errors found.

Shouldn't the socket_listener try to accept the metrics that it received and also that the influx parser should send back the errors as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore linter ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants