Skip to content

Commit

Permalink
[receiver/couchdb] Pass pointer to ScrapeErrors instead of by value (o…
Browse files Browse the repository at this point in the history
…pen-telemetry#13011)

- fixes issue where partial errors are not recorded due to passing the ScrapeErrors struct by value
- rename errors -> errs to avoid possible shadowing of the errors package
  • Loading branch information
BinaryFissionGames committed Aug 9, 2022
1 parent baf4a8c commit 8ad1dcb
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 34 deletions.
48 changes: 24 additions & 24 deletions receiver/couchdbreceiver/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,139 +23,139 @@ import (
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/couchdbreceiver/internal/metadata"
)

func (c *couchdbScraper) recordCouchdbAverageRequestTimeDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbAverageRequestTimeDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
averageRequestTimeMetricKey := []string{"request_time", "value", "arithmetic_mean"}
averageRequestTimeValue, err := getValueFromBody(averageRequestTimeMetricKey, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}

parsedValue, err := c.parseFloat(averageRequestTimeValue)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}
c.mb.RecordCouchdbAverageRequestTimeDataPoint(now, parsedValue)
}

func (c *couchdbScraper) recordCouchdbHttpdBulkRequestsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbHttpdBulkRequestsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
httpdBulkRequestsMetricKey := []string{"httpd", "bulk_requests", "value"}
httpdBulkRequestsMetricValue, err := getValueFromBody(httpdBulkRequestsMetricKey, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}

parsedValue, err := c.parseInt(httpdBulkRequestsMetricValue)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}
c.mb.RecordCouchdbHttpdBulkRequestsDataPoint(now, parsedValue)
}

func (c *couchdbScraper) recordCouchdbHttpdRequestsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbHttpdRequestsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
for methodVal, method := range metadata.MapAttributeHTTPMethod {
httpdRequestMethodKey := []string{"httpd_request_methods", methodVal, "value"}
httpdRequestMethodValue, err := getValueFromBody(httpdRequestMethodKey, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}

parsedValue, err := c.parseInt(httpdRequestMethodValue)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}
c.mb.RecordCouchdbHttpdRequestsDataPoint(now, parsedValue, method)
}
}

func (c *couchdbScraper) recordCouchdbHttpdResponsesDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbHttpdResponsesDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
codes := []string{"200", "201", "202", "204", "206", "301", "302", "304", "400", "401", "403", "404", "405", "406", "409", "412", "413", "414", "415", "416", "417", "500", "501", "503"}
for _, code := range codes {
httpdResponsetCodeKey := []string{"httpd_status_codes", code, "value"}
httpdResponsetCodeValue, err := getValueFromBody(httpdResponsetCodeKey, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}

parsedValue, err := c.parseInt(httpdResponsetCodeValue)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}
c.mb.RecordCouchdbHttpdResponsesDataPoint(now, parsedValue, code)
}
}

func (c *couchdbScraper) recordCouchdbHttpdViewsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbHttpdViewsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
for viewVal, view := range metadata.MapAttributeView {
viewKey := []string{"httpd", viewVal, "value"}
viewValue, err := getValueFromBody(viewKey, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}

parsedValue, err := c.parseInt(viewValue)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}
c.mb.RecordCouchdbHttpdViewsDataPoint(now, parsedValue, view)
}
}

func (c *couchdbScraper) recordCouchdbDatabaseOpenDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbDatabaseOpenDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
openDatabaseKey := []string{"open_databases", "value"}
openDatabaseMetricValue, err := getValueFromBody(openDatabaseKey, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}

parsedValue, err := c.parseInt(openDatabaseMetricValue)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}
c.mb.RecordCouchdbDatabaseOpenDataPoint(now, parsedValue)
}

func (c *couchdbScraper) recordCouchdbFileDescriptorOpenDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbFileDescriptorOpenDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
fileDescriptorKey := []string{"open_os_files", "value"}
fileDescriptorMetricValue, err := getValueFromBody(fileDescriptorKey, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}

parsedValue, err := c.parseInt(fileDescriptorMetricValue)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
return
}
c.mb.RecordCouchdbFileDescriptorOpenDataPoint(now, parsedValue)
}

func (c *couchdbScraper) recordCouchdbDatabaseOperationsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errors scrapererror.ScrapeErrors) {
func (c *couchdbScraper) recordCouchdbDatabaseOperationsDataPoint(now pcommon.Timestamp, stats map[string]interface{}, errs *scrapererror.ScrapeErrors) {
operations := []metadata.AttributeOperation{metadata.AttributeOperationReads, metadata.AttributeOperationWrites}
keyPaths := [][]string{{"database_reads", "value"}, {"database_writes", "value"}}
for i := 0; i < len(operations); i++ {
key := keyPaths[i]
value, err := getValueFromBody(key, stats)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}

parsedValue, err := c.parseInt(value)
if err != nil {
errors.AddPartial(1, err)
errs.AddPartial(1, err)
continue
}
c.mb.RecordCouchdbDatabaseOperationsDataPoint(now, parsedValue, operations[i])
Expand Down
21 changes: 11 additions & 10 deletions receiver/couchdbreceiver/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,16 @@ func (c *couchdbScraper) scrape(context.Context) (pmetric.Metrics, error) {

now := pcommon.NewTimestampFromTime(time.Now())

var errors scrapererror.ScrapeErrors
c.recordCouchdbAverageRequestTimeDataPoint(now, stats, errors)
c.recordCouchdbHttpdBulkRequestsDataPoint(now, stats, errors)
c.recordCouchdbHttpdRequestsDataPoint(now, stats, errors)
c.recordCouchdbHttpdResponsesDataPoint(now, stats, errors)
c.recordCouchdbHttpdViewsDataPoint(now, stats, errors)
c.recordCouchdbDatabaseOpenDataPoint(now, stats, errors)
c.recordCouchdbFileDescriptorOpenDataPoint(now, stats, errors)
c.recordCouchdbDatabaseOperationsDataPoint(now, stats, errors)
errs := &scrapererror.ScrapeErrors{}

return c.mb.Emit(metadata.WithCouchdbNodeName(c.config.Endpoint)), errors.Combine()
c.recordCouchdbAverageRequestTimeDataPoint(now, stats, errs)
c.recordCouchdbHttpdBulkRequestsDataPoint(now, stats, errs)
c.recordCouchdbHttpdRequestsDataPoint(now, stats, errs)
c.recordCouchdbHttpdResponsesDataPoint(now, stats, errs)
c.recordCouchdbHttpdViewsDataPoint(now, stats, errs)
c.recordCouchdbDatabaseOpenDataPoint(now, stats, errs)
c.recordCouchdbFileDescriptorOpenDataPoint(now, stats, errs)
c.recordCouchdbDatabaseOperationsDataPoint(now, stats, errs)

return c.mb.Emit(metadata.WithCouchdbNodeName(c.config.Endpoint)), errs.Combine()
}
17 changes: 17 additions & 0 deletions receiver/couchdbreceiver/scraper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/receiver/scrapererror"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
Expand Down Expand Up @@ -76,6 +78,21 @@ func TestScrape(t *testing.T) {
require.NoError(t, scrapertest.CompareMetrics(expectedMetrics, actualMetrics))
})

t.Run("scrape returns nothing", func(t *testing.T) {
mockClient := new(MockClient)
mockClient.On("GetStats", "_local").Return(map[string]interface{}{}, nil)
scraper := newCouchdbScraper(componenttest.NewNopReceiverCreateSettings(), cfg)
scraper.client = mockClient

metrics, err := scraper.scrape(context.Background())
require.Error(t, err)
assert.Equal(t, 0, metrics.DataPointCount(), "Expected 0 datapoints to be collected")

var partialScrapeErr scrapererror.PartialScrapeError
require.True(t, errors.As(err, &partialScrapeErr), "returned error was not PartialScrapeError")
require.True(t, partialScrapeErr.Failed > 0, "Expected scrape failures, but none were recorded!")
})

t.Run("scrape error: failed to connect to client", func(t *testing.T) {
scraper := newCouchdbScraper(componenttest.NewNopReceiverCreateSettings(), cfg)

Expand Down
12 changes: 12 additions & 0 deletions unreleased/couchdbreceiver_fix-scrape-errors.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: couchdbreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix some partial errors not being correctly reported

# One or more tracking issues related to the change
issues: [13007]

0 comments on commit 8ad1dcb

Please sign in to comment.