From db8569db155bcae11c9c5b1dcc70080b701f4809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mauricio=20V=C3=A1squez?= Date: Wed, 18 Mar 2020 11:34:07 -0500 Subject: [PATCH 1/5] ext/jaeger: fix exporting to collector The exporting of traces to the collector is broken, it replies with the "Unable to process request body: Required field Process is not set" error. The current implementation is based on OpenCensus [1], what appears to be broken too, it's not totally clear at this time what's wrong with that. This commit changes the exporting logic to be similar to the opentelemetry-go [2] one that is working. The main change is to perform the request directly without using the client provided by the generated files. [1] https://github.com/census-instrumentation/opencensus-python/tree/master/contrib/opencensus-ext-jaeger [2] https://github.com/open-telemetry/opentelemetry-go/blob/master/exporters/trace/jaeger/jaeger.go --- ext/opentelemetry-ext-jaeger/setup.cfg | 1 + .../src/opentelemetry/ext/jaeger/__init__.py | 57 +++++-------------- 2 files changed, 16 insertions(+), 42 deletions(-) diff --git a/ext/opentelemetry-ext-jaeger/setup.cfg b/ext/opentelemetry-ext-jaeger/setup.cfg index a5f04f1e9b3..61e0a3059f8 100644 --- a/ext/opentelemetry-ext-jaeger/setup.cfg +++ b/ext/opentelemetry-ext-jaeger/setup.cfg @@ -40,6 +40,7 @@ package_dir= packages=find_namespace: install_requires = thrift >= 0.10.0 + requests opentelemetry-api opentelemetry-sdk diff --git a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py index 6679ce6b7e8..f727cc56b8e 100644 --- a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py +++ b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py @@ -15,10 +15,10 @@ """Jaeger Span Exporter for OpenTelemetry.""" -import base64 import logging import socket +import requests from thrift.protocol import TBinaryProtocol, TCompactProtocol from thrift.transport import THttpClient, TTransport @@ -342,49 +342,22 @@ class Collector: Args: thrift_url: URL of the Jaeger HTTP Thrift. auth: Auth tuple that contains username and password for Basic Auth. - client: Class for creating a Jaeger collector client. - http_transport: Class for creating new client for Thrift HTTP server. """ - def __init__( - self, - thrift_url="", - auth=None, - client=jaeger.Client, - http_transport=THttpClient.THttpClient, - ): + HEADERS = {"Content-Type": "application/x-thrift"} + + def __init__(self, thrift_url="", auth=None): self.thrift_url = thrift_url self.auth = auth - self.http_transport = http_transport(uri_or_host=thrift_url) - self.client = client( - iprot=TBinaryProtocol.TBinaryProtocol(trans=self.http_transport) - ) - # set basic auth header - if auth is not None: - auth_header = "{}:{}".format(*auth) - decoded = base64.b64encode(auth_header.encode()).decode("ascii") - basic_auth = dict(Authorization="Basic {}".format(decoded)) - self.http_transport.setCustomHeaders(basic_auth) - - def submit(self, batch: jaeger.Batch): - """Submits batches to Thrift HTTP Server through Binary Protocol. - - Args: - batch: Object to emit Jaeger spans. - """ - try: - self.client.submitBatches([batch]) - # it will call http_transport.flush() and - # status code and message will be updated - code = self.http_transport.code - msg = self.http_transport.message - if code >= 300 or code < 200: - logger.error( - "Traces cannot be uploaded; HTTP status code: %s, message %s", - code, - msg, - ) - finally: - if self.http_transport.isOpen(): - self.http_transport.close() + def submit(self, batch): + transport = TTransport.TMemoryBuffer() + protocol = TBinaryProtocol.TBinaryProtocol(transport) + batch.write(protocol) + body = transport.getvalue() + requests.post( + url=self.thrift_url, + data=body, + headers=self.HEADERS, + auth=self.auth, + ) From 59ce9ebdecf17d2360362f2b23ecb98b29d0c9d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mauricio=20V=C3=A1squez?= Date: Wed, 18 Mar 2020 15:00:00 -0500 Subject: [PATCH 2/5] ext/jaeger: fix exporting to collector The exporting of traces to the collector is broken, it replies with the "Unable to process request body: Required field Process is not set" error. The current implementation is based on OpenCensus [1], what appears to be broken too, it's not totally clear at this time what's wrong with that. This commit changes the logic to avoid using jaeger.Client that appears to be the broken piece, it is changed by a direct invokation to the THTTPClient transport from thrift. --- ext/opentelemetry-ext-jaeger/setup.cfg | 1 - .../src/opentelemetry/ext/jaeger/__init__.py | 27 ++++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ext/opentelemetry-ext-jaeger/setup.cfg b/ext/opentelemetry-ext-jaeger/setup.cfg index 61e0a3059f8..a5f04f1e9b3 100644 --- a/ext/opentelemetry-ext-jaeger/setup.cfg +++ b/ext/opentelemetry-ext-jaeger/setup.cfg @@ -40,7 +40,6 @@ package_dir= packages=find_namespace: install_requires = thrift >= 0.10.0 - requests opentelemetry-api opentelemetry-sdk diff --git a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py index f727cc56b8e..0f0a92d954f 100644 --- a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py +++ b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py @@ -15,10 +15,10 @@ """Jaeger Span Exporter for OpenTelemetry.""" +import base64 import logging import socket -import requests from thrift.protocol import TBinaryProtocol, TCompactProtocol from thrift.transport import THttpClient, TTransport @@ -344,20 +344,21 @@ class Collector: auth: Auth tuple that contains username and password for Basic Auth. """ - HEADERS = {"Content-Type": "application/x-thrift"} - def __init__(self, thrift_url="", auth=None): self.thrift_url = thrift_url self.auth = auth + self.http_transport = THttpClient.THttpClient( + uri_or_host=self.thrift_url + ) + self.protocol = TBinaryProtocol.TBinaryProtocol(self.http_transport) + + # set basic auth header + if auth is not None: + auth_header = "{}:{}".format(*auth) + decoded = base64.b64encode(auth_header.encode()).decode("ascii") + basic_auth = dict(Authorization="Basic {}".format(decoded)) + self.http_transport.setCustomHeaders(basic_auth) def submit(self, batch): - transport = TTransport.TMemoryBuffer() - protocol = TBinaryProtocol.TBinaryProtocol(transport) - batch.write(protocol) - body = transport.getvalue() - requests.post( - url=self.thrift_url, - data=body, - headers=self.HEADERS, - auth=self.auth, - ) + batch.write(self.protocol) + self.http_transport.flush() From 90448d3a581d75c607fedce51b8166732d3e9bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mauricio=20V=C3=A1squez?= Date: Wed, 18 Mar 2020 15:15:15 -0500 Subject: [PATCH 3/5] read missing check on status code --- .../src/opentelemetry/ext/jaeger/__init__.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py index 0f0a92d954f..9f771abc770 100644 --- a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py +++ b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py @@ -362,3 +362,11 @@ def __init__(self, thrift_url="", auth=None): def submit(self, batch): batch.write(self.protocol) self.http_transport.flush() + code = self.http_transport.code + msg = self.http_transport.message + if code >= 300 or code < 200: + logger.error( + "Traces cannot be uploaded; HTTP status code: %s, message: %s", + code, + msg, + ) From fec162426cb76cdf83ce2cfd7eabdf415a8d07bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mauricio=20V=C3=A1squez?= Date: Wed, 18 Mar 2020 15:18:22 -0500 Subject: [PATCH 4/5] restore type annotation --- .../src/opentelemetry/ext/jaeger/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py index 9f771abc770..b81e75737e8 100644 --- a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py +++ b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py @@ -359,7 +359,7 @@ def __init__(self, thrift_url="", auth=None): basic_auth = dict(Authorization="Basic {}".format(decoded)) self.http_transport.setCustomHeaders(basic_auth) - def submit(self, batch): + def submit(self, batch: jaeger.Batch): batch.write(self.protocol) self.http_transport.flush() code = self.http_transport.code From d456d09ba278cecbf08eaf32d0965e47709fe921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mauricio=20V=C3=A1squez?= Date: Wed, 18 Mar 2020 15:19:51 -0500 Subject: [PATCH 5/5] retore function documentation --- .../src/opentelemetry/ext/jaeger/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py index b81e75737e8..6c43294d3eb 100644 --- a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py +++ b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py @@ -360,6 +360,11 @@ def __init__(self, thrift_url="", auth=None): self.http_transport.setCustomHeaders(basic_auth) def submit(self, batch: jaeger.Batch): + """Submits batches to Thrift HTTP Server through Binary Protocol. + + Args: + batch: Object to emit Jaeger spans. + """ batch.write(self.protocol) self.http_transport.flush() code = self.http_transport.code