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/geoipprocessor] Add attributes parameter and consider both source.address and client.address by default #37008

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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/geoipprocessor-client-address.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: Read both the `client.address` and the `source.address` attributes

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

# (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: [user]
2 changes: 1 addition & 1 deletion processor/geoipprocessor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

## Description

The geoIP processor `geoipprocessor` enhances the attributes of a span, log, or metric by appending information about the geographical location of an IP address. To add geographical information, the IP address must be included in the attributes using the [`source.address` semantic conventions key attribute](https://github.com/open-telemetry/semantic-conventions/blob/v1.26.0/docs/general/attributes.md#source). By default, only the resource attributes will be modified. Please refer to [config.go](./config.go) for the config spec.
The geoIP processor `geoipprocessor` enhances the attributes of a span, log, or metric by appending information about the geographical location of an IP address. To add geographical information, the IP address must be included in the attributes using the [`client.address`](https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/general/attributes.md#client-attributes) or the [`source.address`](https://github.com/open-telemetry/semantic-conventions/blob/v1.29.0/docs/general/attributes.md#source) semantic conventions key attribute. By default, only the resource attributes will be modified. Please refer to [config.go](./config.go) for the config spec.

### Geographical location metadata

Expand Down
5 changes: 4 additions & 1 deletion processor/geoipprocessor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ var (
// defaultResourceAttributes holds a list of default resource attribute keys.
// These keys are used to identify an IP address attribute associated with the resource.
defaultResourceAttributes = []attribute.Key{
semconv.SourceAddressKey, // This key represents the standard source address attribute as defined in the OpenTelemetry semantic conventions.
// The client attributes are in use by the HTTP semantic conventions
semconv.ClientAddressKey,
// The source attributes are used when there is no client/server relationship between the two sides, or when that relationship is unknown
semconv.SourceAddressKey,
}
)

Expand Down
63 changes: 29 additions & 34 deletions processor/geoipprocessor/geoip_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"go.opentelemetry.io/collector/processor"
"go.opentelemetry.io/collector/processor/processortest"
"go.opentelemetry.io/otel/attribute"
semconv "go.opentelemetry.io/otel/semconv/v1.21.0"

"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest/plogtest"
Expand Down Expand Up @@ -82,52 +81,49 @@ var baseProviderMock = providerMock{
}

var testCases = []struct {
name string
goldenDir string
context ContextID
lookupAttributes []attribute.Key
name string
goldenDir string
context ContextID
}{
{
name: "default source.address attribute, not found",
goldenDir: "no_source_address",
context: resource,
lookupAttributes: defaultResourceAttributes,
name: "default source.address attribute, not found",
goldenDir: "no_source_address",
context: resource,
},
{
name: "default source.address attribute",
goldenDir: "source_address",
context: resource,
lookupAttributes: defaultResourceAttributes,
name: "default source.address attribute",
goldenDir: "source_address",
context: resource,
},
{
name: "default source.address attribute no geo metadata found by providers",
goldenDir: "source_address_geo_not_found",
context: resource,
lookupAttributes: defaultResourceAttributes,
name: "default source.address attribute no geo metadata found by providers",
goldenDir: "source_address_geo_not_found",
context: resource,
},
{
name: "default source.ip attribute with an unspecified IP address should be skipped",
goldenDir: "unspecified_address",
context: resource,
lookupAttributes: defaultResourceAttributes,
name: "default source.ip attribute with an unspecified IP address should be skipped",
goldenDir: "unspecified_address",
context: resource,
},
{
name: "custom source attributes",
goldenDir: "custom_sources",
context: resource,
lookupAttributes: []attribute.Key{"ip", "host.ip"},
name: "custom source attributes",
goldenDir: "custom_sources",
context: resource,
},
{
name: "do not add resource attributes with an invalid ip",
goldenDir: "invalid_address",
context: resource,
lookupAttributes: defaultResourceAttributes,
name: "do not add resource attributes with an invalid ip",
goldenDir: "invalid_address",
context: resource,
},
{
name: "source address located in inner attributes",
goldenDir: "attribute_source_address",
context: record,
lookupAttributes: defaultResourceAttributes,
name: "source address located in inner attributes",
goldenDir: "attribute_source_address",
context: record,
},
{
name: "client address located in inner attributes",
goldenDir: "attribute_client_address",
context: record,
},
}

Expand Down Expand Up @@ -205,7 +201,6 @@ func TestProcessor(t *testing.T) {
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"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
resourceLogs:
- resource:
attributes:
- key: ip
value:
stringValue: 1.2.2.1
scopeLogs:
- logRecords:
- attributes:
- key: client.address
value:
stringValue: 1.2.3.4
- 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,72 @@
resourceMetrics:
- resource:
attributes:
- key: ip
value:
stringValue: 1.2.2.1
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
- key: client.address
value:
stringValue: 1.2.3.4
isMonotonic: false
unit: "1"
- name: delta.histogram.test
histogram:
aggregationTemporality: 1
dataPoints:
- explicitBounds: [0.01, 0.1, 1, 10, 100]
timeUnixNano: "1000000"
bucketCounts: [9, 12, 17, 8, 34]
attributes:
- key: client.address
value:
stringValue: 1.2.3.4
- name: summary.test
summary:
dataPoints:
- timeUnixNano: "1000000"
quantileValues:
- quantile: 0.25
value: 50
- quantile: 0.5
value: 20
- quantile: 0.75
value: 75
- quantile: 0.95
value: 10
attributes:
- key: client.address
value:
stringValue: 1.2.3.4
- gauge:
dataPoints:
- asDouble: 345
attributes:
- key: client.address
value:
stringValue: 1.2.3.4
- 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:
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"
- key: ip
value:
stringValue: 1.2.2.1
- key: client.address
value:
stringValue: 1.2.3.4
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,56 @@
resourceLogs:
- resource:
attributes:
- key: ip
value:
stringValue: 1.2.2.1
scopeLogs:
- logRecords:
- attributes:
- key: client.address
value:
stringValue: 1.2.3.4
- key: host.name
value:
stringValue: HOST.ONE
- key: log.file.name
value:
stringValue: one.log
- key: geo.city_name
value:
stringValue: Boxford
- key: geo.continent_code
value:
stringValue: EU
- key: geo.continent_name
value:
stringValue: Europe
- key: geo.country_iso_code
value:
stringValue: GB
- key: geo.country_name
value:
stringValue: United Kingdom
- key: geo.location.lat
value:
doubleValue: 1234
- key: geo.location.lon
value:
doubleValue: 5678
- key: geo.postal_code
value:
stringValue: OX1
- key: geo.region_iso_code
value:
stringValue: WBK
- key: geo.region_name
value:
stringValue: West Berkshire
- key: geo.timezone
value:
stringValue: Europe/London
body:
stringValue: hello one
spanId: ""
traceId: ""
scope: {}
Loading