-
Notifications
You must be signed in to change notification settings - Fork 628
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
Changes from all commits
e436636
8f551c9
f7cdf1c
ce61d45
1e37e5d
d60536f
9b4db03
b02f1d3
b0437e3
11b4a32
daf3c94
6f91702
d1d3342
bd83dc6
5532a3a
4fce7b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
@@ -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()) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,3 +40,7 @@ packages=find_namespace: | |
|
||
[options.packages.find] | ||
where = src | ||
|
||
[options.entry_points] | ||
opentelemetry_instrumentor = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)