Log access denied opening a service at debug level (#4674)

plugins/inputs/win_services/README.md

@@ -1,7 +1,9 @@
-# Telegraf Plugin: win_services
-Input plugin to report Windows services info.
+# Windows Services Input Plugin
+
+Reports information about Windows service status.
+
+Monitoring some services may require running Telegraf with administrator privileges.
 
-It requires that Telegraf must be running under the administrator privileges.
 ### Configuration:
 
 ```toml
@@ -43,9 +45,8 @@ The `startup_mode` field can have the following values:
 
 ### Example Output:
 ```
-* Plugin: inputs.win_services, Collection 1
-> win_services,host=WIN2008R2H401,display_name=Server,service_name=LanmanServer state=4i,startup_mode=2i 1500040669000000000
-> win_services,display_name=Remote\ Desktop\ Services,service_name=TermService,host=WIN2008R2H401 state=1i,startup_mode=3i 1500040669000000000
+win_services,host=WIN2008R2H401,display_name=Server,service_name=LanmanServer state=4i,startup_mode=2i 1500040669000000000
+win_services,display_name=Remote\ Desktop\ Services,service_name=TermService,host=WIN2008R2H401 state=1i,startup_mode=3i 1500040669000000000
 ```
 ### TICK Scripts
 

plugins/inputs/win_services/win_services.go

@@ -4,32 +4,52 @@ package win_services
 
 import (
 	"fmt"
+	"log"
+	"os"
+
 	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/plugins/inputs"
 	"golang.org/x/sys/windows/svc"
 	"golang.org/x/sys/windows/svc/mgr"
 )
 
-//WinService provides interface for svc.Service
+type ServiceErr struct {
+	Message string
+	Service string
+	Err     error
+}
+
+func (e *ServiceErr) Error() string {
+	return fmt.Sprintf("%s: '%s': %v", e.Message, e.Service, e.Err)
+}
+
+func IsPermission(err error) bool {
+	if err, ok := err.(*ServiceErr); ok {
+		return os.IsPermission(err.Err)
+	}
+	return false
+}
+
+// WinService provides interface for svc.Service
 type WinService interface {
 	Close() error
 	Config() (mgr.Config, error)
 	Query() (svc.Status, error)
 }
 
-//WinServiceManagerProvider sets interface for acquiring manager instance, like mgr.Mgr
-type WinServiceManagerProvider interface {
+// ManagerProvider sets interface for acquiring manager instance, like mgr.Mgr
+type ManagerProvider interface {
 	Connect() (WinServiceManager, error)
 }
 
-//WinServiceManager provides interface for mgr.Mgr
+// WinServiceManager provides interface for mgr.Mgr
 type WinServiceManager interface {
 	Disconnect() error
 	OpenService(name string) (WinService, error)
 	ListServices() ([]string, error)
 }
 
-//WinSvcMgr is wrapper for mgr.Mgr implementing WinServiceManager interface
+// WinSvcMgr is wrapper for mgr.Mgr implementing WinServiceManager interface
 type WinSvcMgr struct {
 	realMgr *mgr.Mgr
 }
@@ -45,7 +65,7 @@ func (m *WinSvcMgr) ListServices() ([]string, error) {
 	return m.realMgr.ListServices()
 }
 
-//MgProvider is an implementation of WinServiceManagerProvider interface returning WinSvcMgr
+// MgProvider is an implementation of WinServiceManagerProvider interface returning WinSvcMgr
 type MgProvider struct {
 }
 
@@ -71,7 +91,7 @@ var description = "Input plugin to report Windows services info."
 //WinServices is an implementation if telegraf.Input interface, providing info about Windows Services
 type WinServices struct {
 	ServiceNames []string `toml:"service_names"`
-	mgrProvider  WinServiceManagerProvider
+	mgrProvider  ManagerProvider
 }
 
 type ServiceInfo struct {
@@ -79,7 +99,6 @@ type ServiceInfo struct {
 	DisplayName string
 	State       int
 	StartUpMode int
-	Error       error
 }
 
 func (m *WinServices) Description() string {
@@ -91,93 +110,102 @@ func (m *WinServices) SampleConfig() string {
 }
 
 func (m *WinServices) Gather(acc telegraf.Accumulator) error {
+	scmgr, err := m.mgrProvider.Connect()
+	if err != nil {
+		return fmt.Errorf("Could not open service manager: %s", err)
+	}
+	defer scmgr.Disconnect()
 
-	serviceInfos, err := listServices(m.mgrProvider, m.ServiceNames)
-
+	serviceNames, err := listServices(scmgr, m.ServiceNames)
 	if err != nil {
 		return err
 	}
 
-	for _, service := range serviceInfos {
-		if service.Error == nil {
-			fields := make(map[string]interface{})
-			tags := make(map[string]string)
+	for _, srvName := range serviceNames {
+		service, err := collectServiceInfo(scmgr, srvName)
+		if err != nil {
+			if IsPermission(err) {
+				log.Printf("D! Error in plugin [inputs.win_services]: %v", err)
+			} else {
+				acc.AddError(err)
+			}
+			continue
+		}
 
+		tags := map[string]string{
+			"service_name": service.ServiceName,
+		}
 		//display name could be empty, but still valid service
 		if len(service.DisplayName) > 0 {
 			tags["display_name"] = service.DisplayName
 		}
-			tags["service_name"] = service.ServiceName
 
-			fields["state"] = service.State
-			fields["startup_mode"] = service.StartUpMode
-
-			acc.AddFields("win_services", fields, tags)
-		} else {
-			acc.AddError(service.Error)
+		fields := map[string]interface{}{
+			"state":        service.State,
+			"startup_mode": service.StartUpMode,
 		}
+		acc.AddFields("win_services", fields, tags)
 	}
 
 	return nil
 }
 
-//listServices gathers info about given services. If userServices is empty, it return info about all services on current Windows host.  Any a critical error is returned.
-func listServices(mgrProv WinServiceManagerProvider, userServices []string) ([]ServiceInfo, error) {
-	scmgr, err := mgrProv.Connect()
-	if err != nil {
-		return nil, fmt.Errorf("Could not open service manager: %s", err)
+// listServices returns a list of services to gather.
+func listServices(scmgr WinServiceManager, userServices []string) ([]string, error) {
+	if len(userServices) != 0 {
+		return userServices, nil
 	}
-	defer scmgr.Disconnect()
 
-	var serviceNames []string
-	if len(userServices) == 0 {
-		//Listing service names from system
-		serviceNames, err = scmgr.ListServices()
+	names, err := scmgr.ListServices()
 	if err != nil {
 		return nil, fmt.Errorf("Could not list services: %s", err)
 	}
-	} else {
-		serviceNames = userServices
-	}
-	serviceInfos := make([]ServiceInfo, len(serviceNames))
-
-	for i, srvName := range serviceNames {
-		serviceInfos[i] = collectServiceInfo(scmgr, srvName)
-	}
-
-	return serviceInfos, nil
+	return names, nil
 }
 
-//collectServiceInfo gathers info about a  service from WindowsAPI
-func collectServiceInfo(scmgr WinServiceManager, serviceName string) (serviceInfo ServiceInfo) {
-
-	serviceInfo.ServiceName = serviceName
+// collectServiceInfo gathers info about a service.
+func collectServiceInfo(scmgr WinServiceManager, serviceName string) (*ServiceInfo, error) {
 	srv, err := scmgr.OpenService(serviceName)
 	if err != nil {
-		serviceInfo.Error = fmt.Errorf("Could not open service '%s': %s", serviceName, err)
-		return
+		return nil, &ServiceErr{
+			Message: "could not open service",
+			Service: serviceName,
+			Err:     err,
+		}
 	}
 	defer srv.Close()
 
 	srvStatus, err := srv.Query()
-	if err == nil {
-		serviceInfo.State = int(srvStatus.State)
-	} else {
-		serviceInfo.Error = fmt.Errorf("Could not query service '%s': %s", serviceName, err)
-		//finish collecting info on first found error
-		return
+	if err != nil {
+		return nil, &ServiceErr{
+			Message: "could not query service",
+			Service: serviceName,
+			Err:     err,
+		}
 	}
 
 	srvCfg, err := srv.Config()
-	if err == nil {
-		serviceInfo.DisplayName = srvCfg.DisplayName
-		serviceInfo.StartUpMode = int(srvCfg.StartType)
-	} else {
-		serviceInfo.Error = fmt.Errorf("Could not get config of service '%s': %s", serviceName, err)
+	if err != nil {
+		return nil, &ServiceErr{
+			Message: "could not get config of service",
+			Service: serviceName,
+			Err:     err,
 		}
-	return
+	}
+
+	serviceInfo := &ServiceInfo{
+		ServiceName: serviceName,
+		DisplayName: srvCfg.DisplayName,
+		StartUpMode: int(srvCfg.StartType),
+		State:       int(srvStatus.State),
+	}
+	return serviceInfo, nil
 }
 
 func init() {
-	inputs.Add("win_services", func() telegraf.Input { return &WinServices{mgrProvider: &MgProvider{}} })
+	inputs.Add("win_services", func() telegraf.Input {
+		return &WinServices{
+			mgrProvider: &MgProvider{},
+		}
+	})
 }

plugins/inputs/win_services/win_services_integration_test.go

@@ -4,11 +4,10 @@
 package win_services
 
 import (
-	"github.com/influxdata/telegraf/testutil"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"golang.org/x/sys/windows/svc/mgr"
 	"testing"
+
+	"github.com/influxdata/telegraf/testutil"
+	"github.com/stretchr/testify/require"
 )
 
 var InvalidServices = []string{"XYZ1@", "ZYZ@", "SDF_@#"}
@@ -18,57 +17,30 @@ func TestList(t *testing.T) {
 	if testing.Short() {
 		t.Skip("Skipping integration test in short mode")
 	}
-	services, err := listServices(&MgProvider{}, KnownServices)
+	provider := &MgProvider{}
+	scmgr, err := provider.Connect()
 	require.NoError(t, err)
-	assert.Len(t, services, 2, "Different number of services")
-	assert.Equal(t, services[0].ServiceName, KnownServices[0])
-	assert.Nil(t, services[0].Error)
-	assert.Equal(t, services[1].ServiceName, KnownServices[1])
-	assert.Nil(t, services[1].Error)
+	defer scmgr.Disconnect()
+
+	services, err := listServices(scmgr, KnownServices)
+	require.NoError(t, err)
+	require.Len(t, services, 2, "Different number of services")
+	require.Equal(t, services[0], KnownServices[0])
+	require.Equal(t, services[1], KnownServices[1])
 }
 
 func TestEmptyList(t *testing.T) {
 	if testing.Short() {
 		t.Skip("Skipping integration test in short mode")
 	}
-	services, err := listServices(&MgProvider{}, []string{})
+	provider := &MgProvider{}
+	scmgr, err := provider.Connect()
 	require.NoError(t, err)
-	assert.Condition(t, func() bool { return len(services) > 20 }, "Too few service")
-}
+	defer scmgr.Disconnect()
 
-func TestListEr(t *testing.T) {
-	if testing.Short() {
-		t.Skip("Skipping integration test in short mode")
-	}
-	services, err := listServices(&MgProvider{}, InvalidServices)
+	services, err := listServices(scmgr, []string{})
 	require.NoError(t, err)
-	assert.Len(t, services, 3, "Different number of services")
-	for i := 0; i < 3; i++ {
-		assert.Equal(t, services[i].ServiceName, InvalidServices[i])
-		assert.NotNil(t, services[i].Error)
-	}
-}
-
-func TestGather(t *testing.T) {
-	if testing.Short() {
-		t.Skip("Skipping integration test in short mode")
-	}
-	ws := &WinServices{KnownServices, &MgProvider{}}
-	assert.Len(t, ws.ServiceNames, 2, "Different number of services")
-	var acc testutil.Accumulator
-	require.NoError(t, ws.Gather(&acc))
-	assert.Len(t, acc.Errors, 0, "There should be no errors after gather")
-
-	for i := 0; i < 2; i++ {
-		fields := make(map[string]interface{})
-		tags := make(map[string]string)
-		si := getServiceInfo(KnownServices[i])
-		fields["state"] = int(si.State)
-		fields["startup_mode"] = int(si.StartUpMode)
-		tags["service_name"] = si.ServiceName
-		tags["display_name"] = si.DisplayName
-		acc.AssertContainsTaggedFields(t, "win_services", fields, tags)
-	}
+	require.Condition(t, func() bool { return len(services) > 20 }, "Too few service")
 }
 
 func TestGatherErrors(t *testing.T) {
@@ -76,40 +48,8 @@ func TestGatherErrors(t *testing.T) {
 		t.Skip("Skipping integration test in short mode")
 	}
 	ws := &WinServices{InvalidServices, &MgProvider{}}
-	assert.Len(t, ws.ServiceNames, 3, "Different number of services")
+	require.Len(t, ws.ServiceNames, 3, "Different number of services")
 	var acc testutil.Accumulator
 	require.NoError(t, ws.Gather(&acc))
-	assert.Len(t, acc.Errors, 3, "There should be 3 errors after gather")
-}
-
-func getServiceInfo(srvName string) *ServiceInfo {
-
-	scmgr, err := mgr.Connect()
-	if err != nil {
-		return nil
-	}
-	defer scmgr.Disconnect()
-
-	srv, err := scmgr.OpenService(srvName)
-	if err != nil {
-		return nil
-	}
-	var si ServiceInfo
-	si.ServiceName = srvName
-	srvStatus, err := srv.Query()
-	if err == nil {
-		si.State = int(srvStatus.State)
-	} else {
-		si.Error = err
-	}
-
-	srvCfg, err := srv.Config()
-	if err == nil {
-		si.DisplayName = srvCfg.DisplayName
-		si.StartUpMode = int(srvCfg.StartType)
-	} else {
-		si.Error = err
-	}
-	srv.Close()
-	return &si
+	require.Len(t, acc.Errors, 3, "There should be 3 errors after gather")
 }