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

[processor/geoip] skip error if geo metadata is not found #35278

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/skip_if_geo_not_found.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: enhancement

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: No longer return an error when geo metadata is not found by a provider.

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

# (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: []
6 changes: 3 additions & 3 deletions processor/geoipprocessor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func createMetricsProcessor(ctx context.Context, set processor.Settings, cfg com
if err != nil {
return nil, err
}
return processorhelper.NewMetricsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers).processMetrics, processorhelper.WithCapabilities(processorCapabilities))
return processorhelper.NewMetricsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers, set).processMetrics, processorhelper.WithCapabilities(processorCapabilities))
}

func createTracesProcessor(ctx context.Context, set processor.Settings, cfg component.Config, nextConsumer consumer.Traces) (processor.Traces, error) {
Expand All @@ -98,7 +98,7 @@ func createTracesProcessor(ctx context.Context, set processor.Settings, cfg comp
if err != nil {
return nil, err
}
return processorhelper.NewTracesProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers).processTraces, processorhelper.WithCapabilities(processorCapabilities))
return processorhelper.NewTracesProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers, set).processTraces, processorhelper.WithCapabilities(processorCapabilities))
}

func createLogsProcessor(ctx context.Context, set processor.Settings, cfg component.Config, nextConsumer consumer.Logs) (processor.Logs, error) {
Expand All @@ -107,5 +107,5 @@ func createLogsProcessor(ctx context.Context, set processor.Settings, cfg compon
if err != nil {
return nil, err
}
return processorhelper.NewLogsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers).processLogs, processorhelper.WithCapabilities(processorCapabilities))
return processorhelper.NewLogsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers, set).processLogs, processorhelper.WithCapabilities(processorCapabilities))
}
15 changes: 12 additions & 3 deletions processor/geoipprocessor/geoip_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"net"

"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/processor"
"go.opentelemetry.io/otel/attribute"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/geoipprocessor/internal/provider"
)
Expand All @@ -26,15 +28,17 @@ var (
type geoIPProcessor struct {
providers []provider.GeoIPProvider
resourceAttributes []attribute.Key
logger *zap.Logger

cfg *Config
}

func newGeoIPProcessor(processorConfig *Config, resourceAttributes []attribute.Key, providers []provider.GeoIPProvider) *geoIPProcessor {
func newGeoIPProcessor(processorConfig *Config, resourceAttributes []attribute.Key, providers []provider.GeoIPProvider, params processor.Settings) *geoIPProcessor {
return &geoIPProcessor{
resourceAttributes: resourceAttributes,
providers: providers,
cfg: processorConfig,
logger: params.Logger,
}
}

Expand Down Expand Up @@ -70,9 +74,14 @@ func ipFromAttributes(attributes []attribute.Key, resource pcommon.Map) (net.IP,
// It returns a set of attributes containing the geolocation data, or an error if the location could not be determined.
func (g *geoIPProcessor) geoLocation(ctx context.Context, ip net.IP) (attribute.Set, error) {
allAttributes := &attribute.Set{}
for _, provider := range g.providers {
geoAttributes, err := provider.Location(ctx, ip)
for _, geoProvider := range g.providers {
geoAttributes, err := geoProvider.Location(ctx, ip)
if err != nil {
// continue if no metadata is found
if errors.Is(err, provider.ErrNoMetadataFound) {
g.logger.Debug(err.Error(), zap.String("IP", ip.String()))
continue
}
return attribute.Set{}, err
}
*allAttributes = attribute.NewSet(append(allAttributes.ToSlice(), geoAttributes.ToSlice()...)...)
Expand Down
39 changes: 24 additions & 15 deletions processor/geoipprocessor/geoip_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ var testCases = []struct {
context: resource,
lookupAttributes: defaultResourceAttributes,
},
{
name: "default source.address attribute no geo metadata found by providers",
goldenDir: "source_address_geo_not_found",
context: resource,
lookupAttributes: defaultResourceAttributes,
},
{
name: "default source.ip attribute with an unspecified IP address should be skipped",
goldenDir: "unspecified_address",
Expand Down Expand Up @@ -196,21 +202,24 @@ func TestProcessor(t *testing.T) {
return &baseProviderMock, nil
}

baseProviderMock.LocationF = func(context.Context, net.IP) (attribute.Set, error) {
return attribute.NewSet([]attribute.KeyValue{
semconv.SourceAddress("1.2.3.4"),
attribute.String(conventions.AttributeGeoCityName, "Boxford"),
attribute.String(conventions.AttributeGeoContinentCode, "EU"),
attribute.String(conventions.AttributeGeoContinentName, "Europe"),
attribute.String(conventions.AttributeGeoCountryIsoCode, "GB"),
attribute.String(conventions.AttributeGeoCountryName, "United Kingdom"),
attribute.String(conventions.AttributeGeoTimezone, "Europe/London"),
attribute.String(conventions.AttributeGeoRegionIsoCode, "WBK"),
attribute.String(conventions.AttributeGeoRegionName, "West Berkshire"),
attribute.String(conventions.AttributeGeoPostalCode, "OX1"),
attribute.Float64(conventions.AttributeGeoLocationLat, 1234),
attribute.Float64(conventions.AttributeGeoLocationLon, 5678),
}...), nil
baseProviderMock.LocationF = func(_ context.Context, sourceIP net.IP) (attribute.Set, error) {
if sourceIP.Equal(net.IPv4(1, 2, 3, 4)) {
return attribute.NewSet([]attribute.KeyValue{
semconv.SourceAddress("1.2.3.4"),
attribute.String(conventions.AttributeGeoCityName, "Boxford"),
attribute.String(conventions.AttributeGeoContinentCode, "EU"),
attribute.String(conventions.AttributeGeoContinentName, "Europe"),
attribute.String(conventions.AttributeGeoCountryIsoCode, "GB"),
attribute.String(conventions.AttributeGeoCountryName, "United Kingdom"),
attribute.String(conventions.AttributeGeoTimezone, "Europe/London"),
attribute.String(conventions.AttributeGeoRegionIsoCode, "WBK"),
attribute.String(conventions.AttributeGeoRegionName, "West Berkshire"),
attribute.String(conventions.AttributeGeoPostalCode, "OX1"),
attribute.Float64(conventions.AttributeGeoLocationLat, 1234),
attribute.Float64(conventions.AttributeGeoLocationLon, 5678),
}...), nil
}
return attribute.Set{}, provider.ErrNoMetadataFound
}
const providerKey string = "mock"
providerFactories[providerKey] = &baseMockFactory
Expand Down
2 changes: 1 addition & 1 deletion processor/geoipprocessor/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ require (
go.opentelemetry.io/collector/processor v0.110.0
go.opentelemetry.io/otel v1.30.0
go.uber.org/goleak v1.3.0
go.uber.org/zap v1.27.0
)

require go.opentelemetry.io/collector/pdata/pprofile v0.110.0 // indirect
Expand Down Expand Up @@ -108,7 +109,6 @@ require (
go.opentelemetry.io/otel/trace v1.30.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.0 // indirect
go4.org/netipx v0.0.0-20230824141953-6213f710f925 // indirect
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
golang.org/x/net v0.29.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions processor/geoipprocessor/internal/provider/geoipprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ package provider // import "github.com/open-telemetry/opentelemetry-collector-co

import (
"context"
"errors"
"net"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/processor"
"go.opentelemetry.io/otel/attribute"
)

// ErrNoMetadataFound error should be returned when a provider could not find the corresponding IP metadata
var ErrNoMetadataFound = errors.New("no geo IP metadata found")

// Config is the configuration of a GeoIPProvider.
type Config interface {
component.ConfigValidator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ var (
geoIP2CityDBType = "GeoIP2-City"
geoLite2CityDBType = "GeoLite2-City"

errUnsupportedDB = errors.New("unsupported geo IP database type")
errNoMetadataFound = errors.New("no geo IP metadata found")
errUnsupportedDB = errors.New("unsupported geo IP database type")
)

type maxMindProvider struct {
Expand Down Expand Up @@ -51,7 +50,7 @@ func (g *maxMindProvider) Location(_ context.Context, ipAddress net.IP) (attribu
if err != nil {
return attribute.Set{}, err
} else if len(*attrs) == 0 {
return attribute.Set{}, errNoMetadataFound
return attribute.Set{}, provider.ErrNoMetadataFound
}
return attribute.NewSet(*attrs...), nil
default:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
resourceLogs:
- resource:
attributes:
- key: source.address
value:
stringValue: 1.2.3.5
- key: ip
value:
stringValue: 1.2.2.1
scopeLogs:
- logRecords:
- attributes:
- key: host.name
value:
stringValue: HOST.ONE
- key: log.file.name
value:
stringValue: one.log
body:
stringValue: hello one
spanId: ""
traceId: ""
scope: {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
resourceMetrics:
- resource:
attributes:
- key: ip
value:
stringValue: 1.2.2.1
- key: source.address
value:
stringValue: 1.2.3.5
schemaUrl: https://test-res-schema.com/schema
scopeMetrics:
- metrics:
- description: This also isn't a real metric
name: storage.amplitude
sum:
aggregationTemporality: 2
dataPoints:
- asInt: "0"
attributes:
- key: a
value:
stringValue: AAAA
isMonotonic: false
unit: "1"
- name: delta.histogram.test
histogram:
aggregationTemporality: 1
dataPoints:
- explicitBounds: [0.01, 0.1, 1, 10, 100]
bucketCounts: [9, 12, 17, 8, 34]
attributes:
- key: aaa
value:
stringValue: bbb
- name: summary.test
summary:
dataPoints:
- quantileValues:
- quantile: 0.25
value: 50
- quantile: 0.5
value: 20
- quantile: 0.75
value: 75
- quantile: 0.95
value: 10
- gauge:
dataPoints:
- asDouble: 345
attributes:
- key: aaa
value:
stringValue: bbb
timeUnixNano: "1000000"
name: test.gauge
schemaUrl: https://test-scope-schema.com/schema
scope:
attributes:
- key: foo
value:
stringValue: bar
name: MyTestInstrument
version: 1.2.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
resourceSpans:
- resource:
attributes:
- key: source.address
value:
stringValue: 1.2.3.5
- key: ip
value:
stringValue: 1.2.2.1
scopeSpans:
- scope: {}
spans:
- attributes:
- key: http.request.method
value:
stringValue: POST
- key: url.full
value:
stringValue: https://www.foo.bar/search?q=OpenTelemetry#SemConv
- key: http.response.status_code
value:
intValue: "201"
endTimeUnixNano: "1581452773000000789"
events:
- attributes:
- key: event.attr1
value:
stringValue: foo2
- key: event.attr2
value:
stringValue: bar2
name: event2
timeUnixNano: "1581452773000000123"
name: span-elastic-http
parentSpanId: bcff497b5a47310f
spanId: ""
startTimeUnixNano: "1581452772000000321"
status: {}
traceId: ""
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
resourceLogs:
- resource:
attributes:
- key: source.address
value:
stringValue: 1.2.3.5
- key: ip
value:
stringValue: 1.2.2.1
scopeLogs:
- logRecords:
- attributes:
- key: host.name
value:
stringValue: HOST.ONE
- key: log.file.name
value:
stringValue: one.log
body:
stringValue: hello one
spanId: ""
traceId: ""
scope: {}
Loading