-
Notifications
You must be signed in to change notification settings - Fork 148
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
[WIP] fluentd exporter #42
Conversation
…en policy CMP0079 is set to NEW.
…piler defines for API
…y-cpp-contrib into maxgolov/fluentd_exporter
Thanks @maxgolov will have a look. |
I added async uploader with batching queue, configurable reconnection retries, and configurable wait time between batch uploads. Plus E2E tests. E2E test starts up "fluentd-alike" listener, that accepts MsgPack encoded fluentd forward protocol. And verifies that expected number of events have arrived. It also prints the JSON representation of MsgPack in console. E2E tests depend on another PR in the main repo, that implements tests socket server for TCP, UDP and Unix Domain sockets. Test server included here is not intended to be used in production, for testing only, and fundamentally it's an improvement on top of HTTP server we use elsewhere. Now it supports not only HTTP, but raw TCP, UDP and Unix Domain cross-plat. |
@echo off | ||
pushd %~dp0 | ||
echo Building docker image in %CD%... | ||
docker build --rm -t opentelemetry-fluentd ./ |
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.
Why do we need to check in fluentd docker files here? We can just document how to invoke fluentd docker container - either by pulling from Dockerhub or cloning from fluentd repo. This will ensure the latest images are used?
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.
Also, the example dir should ideally contain the example to create a fluentd exporter?
-- Build files have been written to: /home/<user>/source/opentelemetry-cpp/build | ||
$ | ||
``` | ||
|
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.
Will it build successfully if cmake
is involved directly from the contrib repo, assuming that the opentelemetry-cpp
is already installed? If yes, would be good to document that here.
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.
No, that is not how it is supposed to be built. It is expected to be built as an overlay on top of main repo.
LOG_TRACE("sending %zu Span event(s)", obj[1].size()); | ||
std::vector<uint8_t> msg = nlohmann::json::to_msgpack(obj); | ||
// Schedule upload of Span event(s) | ||
Enqueue(msg); |
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.
So, we are batching the events here, and sending them through a separate thread, to optimize the number of TCP connections? As per the specs, the Exporter::Export()
method should really be doing the export through the transmission channels, and the return status should be the actual status of transmission: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#exportbatch
There is already a batch span processor to handle any such batching needs.
Closing this PR. Plan is to consolidate the changes in this PR along with the socket library introduced in open-telemetry/opentelemetry-cpp#871 and raise common PR. |
@lalitb - Draft implementation of
fluentd
exporter.What I'm still working on:
ability to rename "standard" fields, since semantics of the fields are different in Azure MDSD:
Azure/azure-linux-extensions@5ab581b
Microsoft.OSTCExtensions.LinuxDiagnostic extension failing: libicuuc.so.52: cannot open shared object file Azure/WALinuxAgent#148
And I'd like to keep the code entirely vendor-agnostic, i.e. with ability to customize the "standard" field names. This will be done similar to how it's done in ETW exporter, by allowing custom header that provides a set of compile-time constants (macros) that define the field names.
need to rework the individual event storage model from
json object
tostd::list
or tostd::map<std::string,AttributeValue>
, that'll be better for performance than storing all events in intermediate JSON.PR includes the test setup needed to run a test fluent agent:
fluentd
orfluentbit
, both work. It also works with specific of MDSD custom protocol. MDSD itself is available in open Azure Container Registry. I can elaborate on that setup (how to use this exporter to send telemetry to Azure mdsd via fluent) in a separate document.