Skip to content

Commit a91d8eb

Browse files
rogercolljriguera
rogercoll
authored and
jriguera
committed
[processor/geoip] skip error if geo metadata is not found (open-telemetry#35278)
**Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> If a provider does not found any associated metadata to the given IP, the processor will continue the processing instead of returning the error. Nonetheless, the error will be logged when debug telemetry level is enabled. **Link to tracking Issue:** <Issue number if applicable> open-telemetry#35047 **Testing:** <Describe what testing was performed and which tests were added.> Add a testdata case for IP `1.2.3.5` which is not available in any of the providers (maxmind neither mocked provider) **Documentation:** <Describe the documentation added.>
1 parent e6238bc commit a91d8eb

File tree

13 files changed

+332
-25
lines changed

13 files changed

+332
-25
lines changed

‎.chloggen/skip_if_geo_not_found.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: geoipprocessor
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: No longer return an error when geo metadata is not found by a provider.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [35047]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

‎processor/geoipprocessor/factory.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func createMetricsProcessor(ctx context.Context, set processor.Settings, cfg com
8989
if err != nil {
9090
return nil, err
9191
}
92-
return processorhelper.NewMetricsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers).processMetrics, processorhelper.WithCapabilities(processorCapabilities))
92+
return processorhelper.NewMetricsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers, set).processMetrics, processorhelper.WithCapabilities(processorCapabilities))
9393
}
9494

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

104104
func createLogsProcessor(ctx context.Context, set processor.Settings, cfg component.Config, nextConsumer consumer.Logs) (processor.Logs, error) {
@@ -107,5 +107,5 @@ func createLogsProcessor(ctx context.Context, set processor.Settings, cfg compon
107107
if err != nil {
108108
return nil, err
109109
}
110-
return processorhelper.NewLogsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers).processLogs, processorhelper.WithCapabilities(processorCapabilities))
110+
return processorhelper.NewLogsProcessor(ctx, set, cfg, nextConsumer, newGeoIPProcessor(geoCfg, defaultResourceAttributes, providers, set).processLogs, processorhelper.WithCapabilities(processorCapabilities))
111111
}

‎processor/geoipprocessor/geoip_processor.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import (
1010
"net"
1111

1212
"go.opentelemetry.io/collector/pdata/pcommon"
13+
"go.opentelemetry.io/collector/processor"
1314
"go.opentelemetry.io/otel/attribute"
15+
"go.uber.org/zap"
1416

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

3033
cfg *Config
3134
}
3235

33-
func newGeoIPProcessor(processorConfig *Config, resourceAttributes []attribute.Key, providers []provider.GeoIPProvider) *geoIPProcessor {
36+
func newGeoIPProcessor(processorConfig *Config, resourceAttributes []attribute.Key, providers []provider.GeoIPProvider, params processor.Settings) *geoIPProcessor {
3437
return &geoIPProcessor{
3538
resourceAttributes: resourceAttributes,
3639
providers: providers,
3740
cfg: processorConfig,
41+
logger: params.Logger,
3842
}
3943
}
4044

@@ -70,9 +74,14 @@ func ipFromAttributes(attributes []attribute.Key, resource pcommon.Map) (net.IP,
7074
// It returns a set of attributes containing the geolocation data, or an error if the location could not be determined.
7175
func (g *geoIPProcessor) geoLocation(ctx context.Context, ip net.IP) (attribute.Set, error) {
7276
allAttributes := &attribute.Set{}
73-
for _, provider := range g.providers {
74-
geoAttributes, err := provider.Location(ctx, ip)
77+
for _, geoProvider := range g.providers {
78+
geoAttributes, err := geoProvider.Location(ctx, ip)
7579
if err != nil {
80+
// continue if no metadata is found
81+
if errors.Is(err, provider.ErrNoMetadataFound) {
82+
g.logger.Debug(err.Error(), zap.String("IP", ip.String()))
83+
continue
84+
}
7685
return attribute.Set{}, err
7786
}
7887
*allAttributes = attribute.NewSet(append(allAttributes.ToSlice(), geoAttributes.ToSlice()...)...)

‎processor/geoipprocessor/geoip_processor_test.go

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,12 @@ var testCases = []struct {
9999
context: resource,
100100
lookupAttributes: defaultResourceAttributes,
101101
},
102+
{
103+
name: "default source.address attribute no geo metadata found by providers",
104+
goldenDir: "source_address_geo_not_found",
105+
context: resource,
106+
lookupAttributes: defaultResourceAttributes,
107+
},
102108
{
103109
name: "default source.ip attribute with an unspecified IP address should be skipped",
104110
goldenDir: "unspecified_address",
@@ -196,21 +202,24 @@ func TestProcessor(t *testing.T) {
196202
return &baseProviderMock, nil
197203
}
198204

199-
baseProviderMock.LocationF = func(context.Context, net.IP) (attribute.Set, error) {
200-
return attribute.NewSet([]attribute.KeyValue{
201-
semconv.SourceAddress("1.2.3.4"),
202-
attribute.String(conventions.AttributeGeoCityName, "Boxford"),
203-
attribute.String(conventions.AttributeGeoContinentCode, "EU"),
204-
attribute.String(conventions.AttributeGeoContinentName, "Europe"),
205-
attribute.String(conventions.AttributeGeoCountryIsoCode, "GB"),
206-
attribute.String(conventions.AttributeGeoCountryName, "United Kingdom"),
207-
attribute.String(conventions.AttributeGeoTimezone, "Europe/London"),
208-
attribute.String(conventions.AttributeGeoRegionIsoCode, "WBK"),
209-
attribute.String(conventions.AttributeGeoRegionName, "West Berkshire"),
210-
attribute.String(conventions.AttributeGeoPostalCode, "OX1"),
211-
attribute.Float64(conventions.AttributeGeoLocationLat, 1234),
212-
attribute.Float64(conventions.AttributeGeoLocationLon, 5678),
213-
}...), nil
205+
baseProviderMock.LocationF = func(_ context.Context, sourceIP net.IP) (attribute.Set, error) {
206+
if sourceIP.Equal(net.IPv4(1, 2, 3, 4)) {
207+
return attribute.NewSet([]attribute.KeyValue{
208+
semconv.SourceAddress("1.2.3.4"),
209+
attribute.String(conventions.AttributeGeoCityName, "Boxford"),
210+
attribute.String(conventions.AttributeGeoContinentCode, "EU"),
211+
attribute.String(conventions.AttributeGeoContinentName, "Europe"),
212+
attribute.String(conventions.AttributeGeoCountryIsoCode, "GB"),
213+
attribute.String(conventions.AttributeGeoCountryName, "United Kingdom"),
214+
attribute.String(conventions.AttributeGeoTimezone, "Europe/London"),
215+
attribute.String(conventions.AttributeGeoRegionIsoCode, "WBK"),
216+
attribute.String(conventions.AttributeGeoRegionName, "West Berkshire"),
217+
attribute.String(conventions.AttributeGeoPostalCode, "OX1"),
218+
attribute.Float64(conventions.AttributeGeoLocationLat, 1234),
219+
attribute.Float64(conventions.AttributeGeoLocationLon, 5678),
220+
}...), nil
221+
}
222+
return attribute.Set{}, provider.ErrNoMetadataFound
214223
}
215224
const providerKey string = "mock"
216225
providerFactories[providerKey] = &baseMockFactory

‎processor/geoipprocessor/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ require (
1717
go.opentelemetry.io/collector/processor v0.110.0
1818
go.opentelemetry.io/otel v1.30.0
1919
go.uber.org/goleak v1.3.0
20+
go.uber.org/zap v1.27.0
2021
)
2122

2223
require go.opentelemetry.io/collector/pdata/pprofile v0.110.0 // indirect
@@ -108,7 +109,6 @@ require (
108109
go.opentelemetry.io/otel/trace v1.30.0 // indirect
109110
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
110111
go.uber.org/multierr v1.11.0 // indirect
111-
go.uber.org/zap v1.27.0 // indirect
112112
go4.org/netipx v0.0.0-20230824141953-6213f710f925 // indirect
113113
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
114114
golang.org/x/net v0.29.0 // indirect

‎processor/geoipprocessor/internal/provider/geoipprovider.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,17 @@ package provider // import "github.com/open-telemetry/opentelemetry-collector-co
55

66
import (
77
"context"
8+
"errors"
89
"net"
910

1011
"go.opentelemetry.io/collector/component"
1112
"go.opentelemetry.io/collector/processor"
1213
"go.opentelemetry.io/otel/attribute"
1314
)
1415

16+
// ErrNoMetadataFound error should be returned when a provider could not find the corresponding IP metadata
17+
var ErrNoMetadataFound = errors.New("no geo IP metadata found")
18+
1519
// Config is the configuration of a GeoIPProvider.
1620
type Config interface {
1721
component.ConfigValidator

‎processor/geoipprocessor/internal/provider/maxmindprovider/provider.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ var (
2222
geoIP2CityDBType = "GeoIP2-City"
2323
geoLite2CityDBType = "GeoLite2-City"
2424

25-
errUnsupportedDB = errors.New("unsupported geo IP database type")
26-
errNoMetadataFound = errors.New("no geo IP metadata found")
25+
errUnsupportedDB = errors.New("unsupported geo IP database type")
2726
)
2827

2928
type maxMindProvider struct {
@@ -51,7 +50,7 @@ func (g *maxMindProvider) Location(_ context.Context, ipAddress net.IP) (attribu
5150
if err != nil {
5251
return attribute.Set{}, err
5352
} else if len(*attrs) == 0 {
54-
return attribute.Set{}, errNoMetadataFound
53+
return attribute.Set{}, provider.ErrNoMetadataFound
5554
}
5655
return attribute.NewSet(*attrs...), nil
5756
default:

‎processor/geoipprocessor/testdata/source_address_geo_not_found/input-logs.yaml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
resourceLogs:
2+
- resource:
3+
attributes:
4+
- key: source.address
5+
value:
6+
stringValue: 1.2.3.5
7+
- key: ip
8+
value:
9+
stringValue: 1.2.2.1
10+
scopeLogs:
11+
- logRecords:
12+
- attributes:
13+
- key: host.name
14+
value:
15+
stringValue: HOST.ONE
16+
- key: log.file.name
17+
value:
18+
stringValue: one.log
19+
body:
20+
stringValue: hello one
21+
spanId: ""
22+
traceId: ""
23+
scope: {}

‎processor/geoipprocessor/testdata/source_address_geo_not_found/input-metrics.yaml

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
resourceMetrics:
2+
- resource:
3+
attributes:
4+
- key: ip
5+
value:
6+
stringValue: 1.2.2.1
7+
- key: source.address
8+
value:
9+
stringValue: 1.2.3.5
10+
schemaUrl: https://test-res-schema.com/schema
11+
scopeMetrics:
12+
- metrics:
13+
- description: This also isn't a real metric
14+
name: storage.amplitude
15+
sum:
16+
aggregationTemporality: 2
17+
dataPoints:
18+
- asInt: "0"
19+
attributes:
20+
- key: a
21+
value:
22+
stringValue: AAAA
23+
isMonotonic: false
24+
unit: "1"
25+
- name: delta.histogram.test
26+
histogram:
27+
aggregationTemporality: 1
28+
dataPoints:
29+
- explicitBounds: [0.01, 0.1, 1, 10, 100]
30+
bucketCounts: [9, 12, 17, 8, 34]
31+
attributes:
32+
- key: aaa
33+
value:
34+
stringValue: bbb
35+
- name: summary.test
36+
summary:
37+
dataPoints:
38+
- quantileValues:
39+
- quantile: 0.25
40+
value: 50
41+
- quantile: 0.5
42+
value: 20
43+
- quantile: 0.75
44+
value: 75
45+
- quantile: 0.95
46+
value: 10
47+
- gauge:
48+
dataPoints:
49+
- asDouble: 345
50+
attributes:
51+
- key: aaa
52+
value:
53+
stringValue: bbb
54+
timeUnixNano: "1000000"
55+
name: test.gauge
56+
schemaUrl: https://test-scope-schema.com/schema
57+
scope:
58+
attributes:
59+
- key: foo
60+
value:
61+
stringValue: bar
62+
name: MyTestInstrument
63+
version: 1.2.3

‎processor/geoipprocessor/testdata/source_address_geo_not_found/input-traces.yaml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
resourceSpans:
2+
- resource:
3+
attributes:
4+
- key: source.address
5+
value:
6+
stringValue: 1.2.3.5
7+
- key: ip
8+
value:
9+
stringValue: 1.2.2.1
10+
scopeSpans:
11+
- scope: {}
12+
spans:
13+
- attributes:
14+
- key: http.request.method
15+
value:
16+
stringValue: POST
17+
- key: url.full
18+
value:
19+
stringValue: https://www.foo.bar/search?q=OpenTelemetry#SemConv
20+
- key: http.response.status_code
21+
value:
22+
intValue: "201"
23+
endTimeUnixNano: "1581452773000000789"
24+
events:
25+
- attributes:
26+
- key: event.attr1
27+
value:
28+
stringValue: foo2
29+
- key: event.attr2
30+
value:
31+
stringValue: bar2
32+
name: event2
33+
timeUnixNano: "1581452773000000123"
34+
name: span-elastic-http
35+
parentSpanId: bcff497b5a47310f
36+
spanId: ""
37+
startTimeUnixNano: "1581452772000000321"
38+
status: {}
39+
traceId: ""

‎processor/geoipprocessor/testdata/source_address_geo_not_found/output-logs.yaml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
resourceLogs:
2+
- resource:
3+
attributes:
4+
- key: source.address
5+
value:
6+
stringValue: 1.2.3.5
7+
- key: ip
8+
value:
9+
stringValue: 1.2.2.1
10+
scopeLogs:
11+
- logRecords:
12+
- attributes:
13+
- key: host.name
14+
value:
15+
stringValue: HOST.ONE
16+
- key: log.file.name
17+
value:
18+
stringValue: one.log
19+
body:
20+
stringValue: hello one
21+
spanId: ""
22+
traceId: ""
23+
scope: {}

0 commit comments

Comments
 (0)