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

ext/jaeger: fix exporting to collector #508

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions ext/opentelemetry-ext-jaeger/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ package_dir=
packages=find_namespace:
install_requires =
thrift >= 0.10.0
requests
opentelemetry-api
opentelemetry-sdk

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

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

Is it not possible to stream this to requests.post? With this, we have to hold the whole serialized blob in memory at once.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I studied the problem a little bit more and realized it is possible to directly use thrift.transport.THttpClient as transport of the TBinaryProtocol object, it means that the writes will be done directly in the buffer used to stream data avoiding this intermediate storage, it also avoid to have requests as a dependency here.

requests.post(
url=self.thrift_url,
data=body,
headers=self.HEADERS,
auth=self.auth,
)