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

Add net.peer.ip in requests & urllib3 instrumentations. #661

Merged
merged 16 commits into from
Sep 27, 2021
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
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
- 'release/*'
pull_request:
env:
CORE_REPO_SHA: c49ad57bfe35cfc69bfa863d74058ca9bec55fc3
CORE_REPO_SHA: d9c22a87b6bfc5ec332588c764f82c32f068b2c3

jobs:
build:
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `opentelemetry-instrumentation-urllib3` Updated `_RequestHookT` with two additional fields - the request body and the request headers
([#660](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/660))

### Added

- `opentelemetry-instrumentation-urllib3`, `opentelemetry-instrumentation-requests`
The `net.peer.ip` attribute is set to the IP of the connected HTTP server or proxy
using a new instrumentor in `opententelemetry-util-http`
([#661](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/661))

## [1.5.0-0.24b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.5.0-0.24b0) - 2021-08-26

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
from opentelemetry.trace import SpanKind, get_tracer
from opentelemetry.trace.status import Status
from opentelemetry.util.http import remove_url_credentials
from opentelemetry.util.http.httplib import set_ip_on_next_http_connection

# A key to a context variable to avoid creating duplicate spans when instrumenting
# both, Session.request and Session.send, since Session.request calls into Session.send
Expand Down Expand Up @@ -133,7 +134,7 @@ def _instrumented_requests_call(

with tracer.start_as_current_span(
span_name, kind=SpanKind.CLIENT
) as span:
) as span, set_ip_on_next_http_connection(span):
exception = None
if span.is_recording():
span.set_attribute(SpanAttributes.HTTP_METHOD, method)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# 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.

import requests

from opentelemetry import trace
from opentelemetry.instrumentation.requests import RequestsInstrumentor
from opentelemetry.test.httptest import HttpTestBase
from opentelemetry.test.test_base import TestBase
from opentelemetry.util.http.httplib import HttpClientInstrumentor


class TestURLLib3InstrumentorWithRealSocket(HttpTestBase, TestBase):
def setUp(self):
super().setUp()
self.assert_ip = self.server.server_address[0]
self.http_host = ":".join(map(str, self.server.server_address[:2]))
self.http_url_base = "http://" + self.http_host
self.http_url = self.http_url_base + "/status/200"
HttpClientInstrumentor().instrument()
RequestsInstrumentor().instrument()

def tearDown(self):
super().tearDown()
HttpClientInstrumentor().uninstrument()
RequestsInstrumentor().uninstrument()

@staticmethod
def perform_request(url: str) -> requests.Response:
return requests.get(url)

def test_basic_http_success(self):
response = self.perform_request(self.http_url)
self.assert_success_span(response)

def test_basic_http_success_using_connection_pool(self):
with requests.Session() as session:
response = session.get(self.http_url)

self.assert_success_span(response)

# Test that when re-using an existing connection, everything still works.
# Especially relevant for IP capturing.
response = session.get(self.http_url)

self.assert_success_span(response)

def assert_span(self, num_spans=1): # TODO: Move this to TestBase
span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(num_spans, len(span_list))
if num_spans == 0:
return None
self.memory_exporter.clear()
if num_spans == 1:
return span_list[0]
return span_list

def assert_success_span(self, response: requests.Response):
self.assertEqual("Hello!", response.text)

span = self.assert_span()
self.assertIs(trace.SpanKind.CLIENT, span.kind)
self.assertEqual("HTTP GET", span.name)

attributes = {
"http.status_code": 200,
"net.peer.ip": self.assert_ip,
}
self.assertGreaterEqual(span.attributes.items(), attributes.items())
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ install_requires =
opentelemetry-api ~= 1.3
opentelemetry-semantic-conventions == 0.24b0
opentelemetry-instrumentation == 0.24b0
opentelemetry-util-http == 0.24b0
wrapt >= 1.0.0, < 2.0.0

[options.extras_require]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def response_hook(span, request, response):
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import Span, SpanKind, get_tracer
from opentelemetry.trace.status import Status
from opentelemetry.util.http.httplib import set_ip_on_next_http_connection

# A key to a context variable to avoid creating duplicate spans when instrumenting
# both, Session.request and Session.send, since Session.request calls into Session.send
Expand Down Expand Up @@ -168,7 +169,7 @@ def instrumented_urlopen(wrapped, instance, args, kwargs):

with tracer.start_as_current_span(
span_name, kind=SpanKind.CLIENT, attributes=span_attributes
) as span:
) as span, set_ip_on_next_http_connection(span):
if callable(request_hook):
request_hook(span, instance, headers, body)
inject(headers)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# 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.

import urllib3
import urllib3.exceptions

from opentelemetry import trace
from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor
from opentelemetry.test.httptest import HttpTestBase
from opentelemetry.test.test_base import TestBase
from opentelemetry.util.http.httplib import HttpClientInstrumentor


class TestURLLib3InstrumentorWithRealSocket(HttpTestBase, TestBase):
def setUp(self):
super().setUp()
self.assert_ip = self.server.server_address[0]
self.http_host = ":".join(map(str, self.server.server_address[:2]))
self.http_url_base = "http://" + self.http_host
self.http_url = self.http_url_base + "/status/200"
HttpClientInstrumentor().instrument()
URLLib3Instrumentor().instrument()

def tearDown(self):
super().tearDown()
HttpClientInstrumentor().uninstrument()
URLLib3Instrumentor().uninstrument()

@staticmethod
def perform_request(url: str) -> urllib3.response.HTTPResponse:
with urllib3.PoolManager() as pool:
resp = pool.request("GET", url)
resp.close()
return resp

def test_basic_http_success(self):
response = self.perform_request(self.http_url)
self.assert_success_span(response, self.http_url)

def test_basic_http_success_using_connection_pool(self):
with urllib3.HTTPConnectionPool(self.http_host, timeout=3) as pool:
response = pool.request("GET", "/status/200")

self.assert_success_span(response, self.http_url)

# Test that when re-using an existing connection, everything still works.
# Especially relevant for IP capturing.
response = pool.request("GET", "/status/200")

self.assert_success_span(response, self.http_url)

def assert_span(self, num_spans=1):
span_list = self.memory_exporter.get_finished_spans()
self.assertEqual(num_spans, len(span_list))
if num_spans == 0:
return None
self.memory_exporter.clear()
if num_spans == 1:
return span_list[0]
return span_list

def assert_success_span(
self, response: urllib3.response.HTTPResponse, url: str
):
self.assertEqual(b"Hello!", response.data)

span = self.assert_span()
self.assertIs(trace.SpanKind.CLIENT, span.kind)
self.assertEqual("HTTP GET", span.name)

attributes = {
"http.status_code": 200,
"net.peer.ip": self.assert_ip,
}
self.assertGreaterEqual(span.attributes.items(), attributes.items())
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ commands_pre =

grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test]

falcon,flask,django,pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test]
falcon,flask,django,pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,urllib3,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test]
wsgi,falcon,flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test]
asgi,starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test]

Expand Down
4 changes: 4 additions & 0 deletions util/opentelemetry-util-http/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ packages=find_namespace:

[options.packages.find]
where = src

[options.entry_points]
opentelemetry_instrumentor =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared to the original PR version with a separate package for this:
On the minus side, this now means that everyone depending on util-http will get the httplib.client instrumentation, even if they don't use it. The overhead should be small though. On the plus side, there is no new package.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benefit of not having a new package outweighs the small overhead I feel :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, is there some integration test that loads the urllib3 or requests instrumentation via the entrypoint, that I could extend to assert on the net.peer.ip attribute being present, so I could verify the wiring is not messed up here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, you are wanting to test if attribute is present if auto-instrumentation is used? If so, I don't believe we have integration tests for those. We currently only have unit tests for auto-instrumentation.

httplib = opentelemetry.util.http.httplib:HttpClientInstrumentor
Loading