Skip to content

Commit

Permalink
Add ScrapeErrors struct to simplify errors usage (#2414)
Browse files Browse the repository at this point in the history
* Add ScrapeErrors struct to simplify errors usage

Add ScrapeErrors that contains a slice with errors and that has methods
to simplify adding new PartialScrapeErrors and regular generic errors.

Use new methods to refactor errors appends in
receiver/hostmetricsreceiver.

Use ScrapeErrors.Combine() in component/componenterror to not create
cycling dependencies in consumererrors package.

* Component error: do not change CombineErrors yet

Leave componenterror.CombineErrors unchanged to not introduce too
many changes in a single PR.

Add a note about the necessity of componenterror.CombineErrors
deprecation.

* ScrapeErrors: do not iterate over all errors

Rename scrapeErrsCount field to failedScrapeCount.

Add failed count in every Add, Addf call of ScrapeErrors and use this
info to call CombineErrors directly instead of iterating over all errors
in ScrapeErrors.

Fix filesystemscraper Scrape function to not cast error into
PartialScrapeError since it will cause panics after combining
ScrapeErrors. Use NewPartialScrapeError instead.

* ScrapeErrors: fix hostmetricsreceiver Scrape err

Cast getProcessMetadata() error into consumererror.PartialScrapeError to
get the valid scrape errors count.

* ScrapeErrors: fix multiMetricScraper.Scrape method

Cast Scrape() error into consumererror.PartialScrapeError to
get the valid scrape errors count and to not skip combined errors.

* ScrapeErrors: check if slice contains partial err

Fix `Combine()` method to check if errs slice contains
`PartialScrapeError` since we can have scrape errors with 0 failed
metrics so checking only `failedScrapeCount` field is not enough.

* Fix changing comments in combineerrors_test.go

Do not change comments in
consumer/consumererror/combineerrors_test.go.

* Component error: apply 5cc0707 changes

Apply 5cc0707 to component/componenterror.

* ScrapeErrors: rename and simplify methods

Use only Add and AddPartial methods.
  • Loading branch information
ozerovandrei authored Feb 19, 2021
1 parent ba720d8 commit 8fda120
Show file tree
Hide file tree
Showing 10 changed files with 211 additions and 194 deletions.
49 changes: 49 additions & 0 deletions consumer/consumererror/scrapeerrors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package consumererror

// ScrapeErrors contains multiple PartialScrapeErrors and can also contain generic errors.
type ScrapeErrors struct {
errs []error
failedScrapeCount int
}

// Add adds a PartialScrapeError with the provided failed count and error.
func (s *ScrapeErrors) AddPartial(failed int, err error) {
s.errs = append(s.errs, NewPartialScrapeError(err, failed))
s.failedScrapeCount += failed
}

// Add adds a regular error.
func (s *ScrapeErrors) Add(err error) {
s.errs = append(s.errs, err)
}

// Combine converts a slice of errors into one error.
// It will return a PartialScrapeError if at least one error in the slice is a PartialScrapeError.
func (s *ScrapeErrors) Combine() error {
partialScrapeErr := false
for _, err := range s.errs {
if IsPartialScrapeError(err) {
partialScrapeErr = true
}
}

if !partialScrapeErr {
return CombineErrors(s.errs)
}

return NewPartialScrapeError(CombineErrors(s.errs), s.failedScrapeCount)
}
117 changes: 117 additions & 0 deletions consumer/consumererror/scrapeerrors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package consumererror

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
)

func TestScrapeErrorsAddPartial(t *testing.T) {
err1 := errors.New("err 1")
err2 := errors.New("err 2")
expected := []error{
PartialScrapeError{error: err1, Failed: 1},
PartialScrapeError{error: err2, Failed: 10},
}

var errs ScrapeErrors
errs.AddPartial(1, err1)
errs.AddPartial(10, err2)
assert.Equal(t, expected, errs.errs)
}

func TestScrapeErrorsAdd(t *testing.T) {
err1 := errors.New("err a")
err2 := errors.New("err b")
expected := []error{err1, err2}

var errs ScrapeErrors
errs.Add(err1)
errs.Add(err2)
assert.Equal(t, expected, errs.errs)
}

func TestScrapeErrorsCombine(t *testing.T) {
testCases := []struct {
errs func() ScrapeErrors
expectedErr string
expectedFailedCount int
expectNil bool
expectedScrape bool
}{
{
errs: func() ScrapeErrors {
var errs ScrapeErrors
return errs
},
expectNil: true,
},
{
errs: func() ScrapeErrors {
var errs ScrapeErrors
errs.AddPartial(10, errors.New("bad scrapes"))
errs.AddPartial(1, fmt.Errorf("err: %s", errors.New("bad scrape")))
return errs
},
expectedErr: "[bad scrapes; err: bad scrape]",
expectedFailedCount: 11,
expectedScrape: true,
},
{
errs: func() ScrapeErrors {
var errs ScrapeErrors
errs.Add(errors.New("bad regular"))
errs.Add(fmt.Errorf("err: %s", errors.New("bad reg")))
return errs
},
expectedErr: "[bad regular; err: bad reg]",
},
{
errs: func() ScrapeErrors {
var errs ScrapeErrors
errs.AddPartial(2, errors.New("bad two scrapes"))
errs.AddPartial(10, fmt.Errorf("%d scrapes failed: %s", 10, errors.New("bad things happened")))
errs.Add(errors.New("bad event"))
errs.Add(fmt.Errorf("event: %s", errors.New("something happened")))
return errs
},
expectedErr: "[bad two scrapes; 10 scrapes failed: bad things happened; bad event; event: something happened]",
expectedFailedCount: 12,
expectedScrape: true,
},
}

for _, tc := range testCases {
scrapeErrs := tc.errs()
if (scrapeErrs.Combine() == nil) != tc.expectNil {
t.Errorf("%+v.Combine() == nil? Got: %t. Want: %t", scrapeErrs, scrapeErrs.Combine() == nil, tc.expectNil)
}
if scrapeErrs.Combine() != nil && tc.expectedErr != scrapeErrs.Combine().Error() {
t.Errorf("%+v.Combine() = %q. Want: %q", scrapeErrs, scrapeErrs.Combine(), tc.expectedErr)
}
if tc.expectedScrape {
partialScrapeErr, ok := scrapeErrs.Combine().(PartialScrapeError)
if !ok {
t.Errorf("%+v.Combine() = %q. Want: PartialScrapeError", scrapeErrs, scrapeErrs.Combine())
} else if tc.expectedFailedCount != partialScrapeErr.Failed {
t.Errorf("%+v.Combine().Failed. Got %d Failed count. Want: %d", scrapeErrs, partialScrapeErr.Failed, tc.expectedFailedCount)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/metadata"
"go.opentelemetry.io/collector/receiver/scraperhelper"
)

const (
Expand Down Expand Up @@ -71,15 +70,15 @@ func (s *scraper) Scrape(_ context.Context) (pdata.MetricSlice, error) {
return metrics, consumererror.NewPartialScrapeError(err, metricsLen)
}

var errors []error
var errors consumererror.ScrapeErrors
usages := make([]*deviceUsage, 0, len(partitions))
for _, partition := range partitions {
if !s.fsFilter.includePartition(partition) {
continue
}
usage, usageErr := s.usage(partition.Mountpoint)
if usageErr != nil {
errors = append(errors, consumererror.NewPartialScrapeError(usageErr, 0))
errors.AddPartial(0, usageErr)
continue
}

Expand All @@ -92,11 +91,9 @@ func (s *scraper) Scrape(_ context.Context) (pdata.MetricSlice, error) {
appendSystemSpecificMetrics(metrics, 1, now, usages)
}

err = scraperhelper.CombineScrapeErrors(errors)
err = errors.Combine()
if err != nil && len(usages) == 0 {
partialErr := err.(consumererror.PartialScrapeError)
partialErr.Failed = metricsLen
err = partialErr
err = consumererror.NewPartialScrapeError(err, metricsLen)
}

return metrics, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"go.opentelemetry.io/collector/internal/processor/filterset"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/metadata"
"go.opentelemetry.io/collector/receiver/scraperhelper"
)

const (
Expand Down Expand Up @@ -85,19 +84,19 @@ func (s *scraper) start(context.Context, component.Host) error {
func (s *scraper) scrape(_ context.Context) (pdata.MetricSlice, error) {
metrics := pdata.NewMetricSlice()

var errors []error
var errors consumererror.ScrapeErrors

err := s.scrapeAndAppendNetworkCounterMetrics(metrics, s.startTime)
if err != nil {
errors = append(errors, err)
errors.AddPartial(networkMetricsLen, err)
}

err = s.scrapeAndAppendNetworkConnectionsMetric(metrics)
if err != nil {
errors = append(errors, err)
errors.AddPartial(connectionsMetricsLen, err)
}

return metrics, scraperhelper.CombineScrapeErrors(errors)
return metrics, errors.Combine()
}

func (s *scraper) scrapeAndAppendNetworkCounterMetrics(metrics pdata.MetricSlice, startTime pdata.TimestampUnixNano) error {
Expand All @@ -106,7 +105,7 @@ func (s *scraper) scrapeAndAppendNetworkCounterMetrics(metrics pdata.MetricSlice
// get total stats only
ioCounters, err := s.ioCounters( /*perNetworkInterfaceController=*/ true)
if err != nil {
return consumererror.NewPartialScrapeError(err, networkMetricsLen)
return err
}

// filter network interfaces by name
Expand Down Expand Up @@ -182,7 +181,7 @@ func (s *scraper) scrapeAndAppendNetworkConnectionsMetric(metrics pdata.MetricSl

connections, err := s.connections("tcp")
if err != nil {
return consumererror.NewPartialScrapeError(err, connectionsMetricsLen)
return err
}

tcpConnectionStatusCounts := getTCPConnectionStatusCounts(connections)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/metadata"
"go.opentelemetry.io/collector/receiver/scraperhelper"
)

const (
Expand Down Expand Up @@ -65,26 +64,26 @@ func (s *scraper) start(context.Context, component.Host) error {
func (s *scraper) scrape(_ context.Context) (pdata.MetricSlice, error) {
metrics := pdata.NewMetricSlice()

var errors []error
var errors consumererror.ScrapeErrors

err := s.scrapeAndAppendPagingUsageMetric(metrics)
if err != nil {
errors = append(errors, err)
errors.AddPartial(pagingUsageMetricsLen, err)
}

err = s.scrapeAndAppendPagingMetrics(metrics)
if err != nil {
errors = append(errors, err)
errors.AddPartial(pagingMetricsLen, err)
}

return metrics, scraperhelper.CombineScrapeErrors(errors)
return metrics, errors.Combine()
}

func (s *scraper) scrapeAndAppendPagingUsageMetric(metrics pdata.MetricSlice) error {
now := internal.TimeToUnixNano(time.Now())
vmem, err := s.virtualMemory()
if err != nil {
return consumererror.NewPartialScrapeError(err, pagingUsageMetricsLen)
return err
}

idx := metrics.Len()
Expand Down Expand Up @@ -114,7 +113,7 @@ func (s *scraper) scrapeAndAppendPagingMetrics(metrics pdata.MetricSlice) error
now := internal.TimeToUnixNano(time.Now())
swap, err := s.swapMemory()
if err != nil {
return consumererror.NewPartialScrapeError(err, pagingMetricsLen)
return err
}

idx := metrics.Len()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/metadata"
"go.opentelemetry.io/collector/receiver/hostmetricsreceiver/internal/perfcounters"
"go.opentelemetry.io/collector/receiver/scraperhelper"
)

const (
Expand Down Expand Up @@ -82,26 +81,26 @@ func (s *scraper) start(context.Context, component.Host) error {
func (s *scraper) scrape(context.Context) (pdata.MetricSlice, error) {
metrics := pdata.NewMetricSlice()

var errors []error
var errors consumererror.ScrapeErrors

err := s.scrapeAndAppendPagingUsageMetric(metrics)
if err != nil {
errors = append(errors, err)
errors.AddPartial(pagingUsageMetricsLen, err)
}

err = s.scrapeAndAppendPagingOperationsMetric(metrics)
if err != nil {
errors = append(errors, err)
errors.AddPartial(pagingMetricsLen, err)
}

return metrics, scraperhelper.CombineScrapeErrors(errors)
return metrics, errors.Combine()
}

func (s *scraper) scrapeAndAppendPagingUsageMetric(metrics pdata.MetricSlice) error {
now := internal.TimeToUnixNano(time.Now())
pageFiles, err := s.pageFileStats()
if err != nil {
return consumererror.NewPartialScrapeError(err, pagingUsageMetricsLen)
return err
}

idx := metrics.Len()
Expand Down Expand Up @@ -137,17 +136,17 @@ func (s *scraper) scrapeAndAppendPagingOperationsMetric(metrics pdata.MetricSlic

counters, err := s.perfCounterScraper.Scrape()
if err != nil {
return consumererror.NewPartialScrapeError(err, pagingMetricsLen)
return err
}

memoryObject, err := counters.GetObject(memory)
if err != nil {
return consumererror.NewPartialScrapeError(err, pagingMetricsLen)
return err
}

memoryCounterValues, err := memoryObject.GetValues(pageReadsPerSec, pageWritesPerSec)
if err != nil {
return consumererror.NewPartialScrapeError(err, pagingMetricsLen)
return err
}

if len(memoryCounterValues) > 0 {
Expand Down
Loading

0 comments on commit 8fda120

Please sign in to comment.