Skip to content

Commit

Permalink
[chore] Restore Lint on Windows (open-telemetry#34656)
Browse files Browse the repository at this point in the history
Lint is not happening for `GOOS=windows` . This change restores lint for
Windows on CI runs.

We may have to fix a bunch of lint issues before being able to merge
this one.

Noticed while looking at the failures from open-telemetry#34639
  • Loading branch information
pjanotti authored and f7o committed Sep 12, 2024
1 parent a0132c2 commit d6e7015
Show file tree
Hide file tree
Showing 12 changed files with 50 additions and 23 deletions.
27 changes: 27 additions & 0 deletions .chloggen/pkg_stanza_operator_input_windows_type_name_change.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: pkg/stanza/operator/input/windows

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Change type name from `EvtRpcLogin` to `EvtRPCLogin`.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [34656]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
4 changes: 2 additions & 2 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
strategy:
fail-fast: false
matrix:
test:
goos:
- windows
- linux
group:
Expand Down Expand Up @@ -115,7 +115,7 @@ jobs:
path: ~/.cache/go-build
key: go-lint-build-${{ matrix.group }}-${{ runner.os }}-${{ hashFiles('**/go.sum') }}
- name: Lint
run: GOOS=${{ matrix.os }} GOARCH=amd64 make -j2 golint GROUP=${{ matrix.group }}
run: GOOS=${{ matrix.goos }} GOARCH=amd64 make -j2 golint GROUP=${{ matrix.group }}
lint:
if: ${{ github.actor != 'dependabot[bot]' && always() }}
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion Makefile.Common
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ $(TOOLS_BIN_DIR):
mkdir -p $@

$(TOOLS_BIN_NAMES): $(TOOLS_BIN_DIR) $(TOOLS_MOD_DIR)/go.mod
cd $(TOOLS_MOD_DIR) && $(GOCMD) build -o $@ -trimpath $(filter %/$(notdir $@),$(TOOLS_PKG_NAMES))
cd $(TOOLS_MOD_DIR) && GOOS="" $(GOCMD) build -o $@ -trimpath $(filter %/$(notdir $@),$(TOOLS_PKG_NAMES))

ADDLICENSE := $(TOOLS_BIN_DIR)/addlicense
MDLINKCHECK := $(TOOLS_BIN_DIR)/markdown-link-check
Expand Down
2 changes: 1 addition & 1 deletion pkg/stanza/fileconsumer/attrs/owner_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ import (
"os"
)

func (r *Resolver) addOwnerInfo(file *os.File, attributes map[string]any) error {
func (r *Resolver) addOwnerInfo(_ *os.File, _ map[string]any) error {
return fmt.Errorf("owner info not implemented for windows")
}
2 changes: 1 addition & 1 deletion pkg/stanza/fileconsumer/file_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ import (
)

// Noop on windows because we close files immediately after reading.
func (m *Manager) readLostFiles(ctx context.Context) {
func (m *Manager) readLostFiles(_ context.Context) {
}
8 changes: 4 additions & 4 deletions pkg/stanza/operator/input/windows/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var (
openSessionProc SyscallProc = api.NewProc("EvtOpenSession")
)

type EvtRpcLogin struct {
type EvtRPCLogin struct {
Server *uint16
User *uint16
Domain *uint16
Expand All @@ -47,8 +47,8 @@ const (
EvtSubscribeStartAtOldestRecord uint32 = 2
// EvtSubscribeStartAfterBookmark is a flag that will subscribe to all events that begin after a bookmark.
EvtSubscribeStartAfterBookmark uint32 = 3
// EvtRpcLoginClass is a flag that indicates the login class.
EvtRpcLoginClass uint32 = 1
// EvtRPCLoginClass is a flag that indicates the login class.
EvtRPCLoginClass uint32 = 1
)

const (
Expand Down Expand Up @@ -162,7 +162,7 @@ func evtFormatMessage(publisherMetadata uintptr, event uintptr, messageID uint32
}

// evtOpenSession is the direct syscall implementation of EvtOpenSession (https://learn.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtopensession)
func evtOpenSession(loginClass uint32, login *EvtRpcLogin, timeout uint32, flags uint32) (windows.Handle, error) {
func evtOpenSession(loginClass uint32, login *EvtRPCLogin, timeout uint32, flags uint32) (windows.Handle, error) {
r0, _, e1 := openSessionProc.Call(uintptr(loginClass), uintptr(unsafe.Pointer(login)), uintptr(timeout), uintptr(flags))
handle := windows.Handle(r0)
if handle == 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/stanza/operator/input/windows/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (m MockProc) Call(a ...uintptr) (uintptr, uintptr, error) {
// SimpleMockProc returns a mock proc that will always return the supplied arguments when called.
func SimpleMockProc(r1 uintptr, r2 uintptr, err syscall.Errno) SyscallProc {
return MockProc{
call: func(a ...uintptr) (uintptr, uintptr, error) {
call: func(_ ...uintptr) (uintptr, uintptr, error) {
return r1, r2, err
},
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/stanza/operator/input/windows/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ func (i *Input) defaultStartRemoteSession() error {
return nil
}

login := EvtRpcLogin{
login := EvtRPCLogin{
Server: windows.StringToUTF16Ptr(i.remote.Server),
User: windows.StringToUTF16Ptr(i.remote.Username),
Password: windows.StringToUTF16Ptr(i.remote.Password),
}

sessionHandle, err := evtOpenSession(EvtRpcLoginClass, &login, 0, 0)
sessionHandle, err := evtOpenSession(EvtRPCLoginClass, &login, 0, 0)
if err != nil {
return fmt.Errorf("failed to open session for server %s: %w", i.remote.Server, err)
}
Expand Down Expand Up @@ -304,7 +304,7 @@ func (i *Input) sendEvent(ctx context.Context, eventXML EventXML) {

entry.Timestamp = eventXML.parseTimestamp()
entry.Severity = eventXML.parseRenderedSeverity()
i.Write(ctx, entry)
_ = i.Write(ctx, entry)
}

// sendEventRaw will send EventRaw as an entry to the operator's output.
Expand All @@ -318,7 +318,7 @@ func (i *Input) sendEventRaw(ctx context.Context, eventRaw EventRaw) {

entry.Timestamp = eventRaw.parseTimestamp()
entry.Severity = eventRaw.parseRenderedSeverity()
i.Write(ctx, entry)
_ = i.Write(ctx, entry)
}

// getBookmarkXML will get the bookmark xml from the offsets database.
Expand Down
4 changes: 2 additions & 2 deletions pkg/stanza/operator/input/windows/input_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestInputStart_RemoteAccessDeniedError(t *testing.T) {
originalEvtSubscribeFunc := evtSubscribeFunc
defer func() { evtSubscribeFunc = originalEvtSubscribeFunc }()

evtSubscribeFunc = func(session uintptr, signalEvent windows.Handle, channelPath *uint16, query *uint16, bookmark uintptr, context uintptr, callback uintptr, flags uint32) (uintptr, error) {
evtSubscribeFunc = func(_ uintptr, _ windows.Handle, _ *uint16, _ *uint16, _ uintptr, _ uintptr, _ uintptr, _ uint32) (uintptr, error) {
return 0, windows.ERROR_ACCESS_DENIED
}

Expand All @@ -109,7 +109,7 @@ func TestInputStart_BadChannelName(t *testing.T) {
originalEvtSubscribeFunc := evtSubscribeFunc
defer func() { evtSubscribeFunc = originalEvtSubscribeFunc }()

evtSubscribeFunc = func(session uintptr, signalEvent windows.Handle, channelPath *uint16, query *uint16, bookmark uintptr, context uintptr, callback uintptr, flags uint32) (uintptr, error) {
evtSubscribeFunc = func(_ uintptr, _ windows.Handle, _ *uint16, _ *uint16, _ uintptr, _ uintptr, _ uintptr, _ uint32) (uintptr, error) {
return 0, windows.ERROR_EVT_CHANNEL_NOT_FOUND
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func getProcessName(_ context.Context, _ processHandle, exePath string) (string,
return filepath.Base(exePath), nil
}

func getProcessCgroup(ctx context.Context, proc processHandle) (string, error) {
func getProcessCgroup(_ context.Context, _ processHandle) (string, error) {
return "", nil
}

Expand Down
2 changes: 1 addition & 1 deletion receiver/iisreceiver/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestIntegration(t *testing.T) {
scraperinttest.NewIntegrationTest(
NewFactory(),
scraperinttest.WithCustomConfig(
func(t *testing.T, cfg component.Config, ci *scraperinttest.ContainerInfo) {
func(_ *testing.T, cfg component.Config, _ *scraperinttest.ContainerInfo) {
rCfg := cfg.(*Config)
rCfg.CollectionInterval = 100 * time.Millisecond
}),
Expand Down
10 changes: 5 additions & 5 deletions receiver/iisreceiver/scraper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package iisreceiver // import "github.com/open-telemetry/opentelemetry-collector

import (
"context"
"fmt"
"errors"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -71,7 +71,7 @@ func TestScrapeFailure(t *testing.T) {
)

expectedError := "failure to collect metric"
mockWatcher, err := newMockWatcherFactory(fmt.Errorf(expectedError))("", "", "")
mockWatcher, err := newMockWatcherFactory(errors.New(expectedError))("", "", "")
require.NoError(t, err)
scraper.totalWatcherRecorders = []watcherRecorder{
{
Expand Down Expand Up @@ -107,7 +107,7 @@ func TestMaxQueueItemAgeScrapeFailure(t *testing.T) {
)

expectedError := "failure to collect metric"
mockWatcher, err := newMockWatcherFactory(fmt.Errorf(expectedError))("", "", "")
mockWatcher, err := newMockWatcherFactory(errors.New(expectedError))("", "", "")
require.NoError(t, err)
scraper.queueMaxAgeWatchers = []instanceWatcher{
{
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestMaxQueueItemAgeNegativeDenominatorScrapeFailure(t *testing.T) {
)

expectedError := "Failed to scrape counter \"counter\": A counter with a negative denominator value was detected.\r\n"
mockWatcher, err := newMockWatcherFactory(fmt.Errorf(expectedError))("", "", "")
mockWatcher, err := newMockWatcherFactory(errors.New(expectedError))("", "", "")
require.NoError(t, err)
scraper.queueMaxAgeWatchers = []instanceWatcher{
{
Expand Down Expand Up @@ -171,7 +171,7 @@ func newMockWatcherFactory(watchErr error) func(string, string,
}

func newMockWatcherFactorFromPath(watchErr error, value float64) func(string) (winperfcounters.PerfCounterWatcher, error) {
return func(s string) (winperfcounters.PerfCounterWatcher, error) {
return func(_ string) (winperfcounters.PerfCounterWatcher, error) {
return &mockPerfCounter{watchErr: watchErr, value: value}, nil
}
}
Expand Down

0 comments on commit d6e7015

Please sign in to comment.