Skip to content

Conversation

@lidavidm
Copy link
Member

@lidavidm lidavidm commented May 6, 2021

As discussed on the mailing list, this is a proof-of-concept that integrates OpenTelemetry for (distributed) tracing. This supports the following:

  • OTel is used as a header-only library by default.
  • Optionally, Arrow will link to OTel and configure it based on an environment variable.

No other instrumentation is added; this just sets up the build, and then we can consider what to instrument (FlightRPC, query engine, etc.) and also integrate more backends (Jaeger, OTLP, etc.).

@github-actions
Copy link

github-actions bot commented May 6, 2021

@lidavidm
Copy link
Member Author

lidavidm commented May 6, 2021

@lidavidm lidavidm force-pushed the arrow-opentelemetry branch 7 times, most recently from 3171eba to 4a53e8f Compare May 14, 2021 19:29
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch 2 times, most recently from 1bbb8d2 to e6e9a98 Compare May 28, 2021 15:24
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch 2 times, most recently from 0704912 to 932aaa0 Compare June 9, 2021 16:50
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch from 932aaa0 to 539a911 Compare June 23, 2021 21:36
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch 2 times, most recently from 5c1f02c to c59a89e Compare July 13, 2021 21:03
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch 2 times, most recently from 6bf10ee to ee7b1e8 Compare September 21, 2021 18:18
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch 2 times, most recently from 929228e to e2a9f4f Compare September 22, 2021 14:28
@lidavidm lidavidm changed the title ARROW-12671: [C++][Python][FlightRPC] WIP: Simple OpenTelemetry integration ARROW-12671: [C++][Python][FlightRPC] Simple OpenTelemetry integration Sep 22, 2021
@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@lidavidm lidavidm changed the title ARROW-12671: [C++][Python][FlightRPC] Simple OpenTelemetry integration ARROW-12671: [C++] Add OpenTelemetry to ThirdpartyToolchain Oct 26, 2021
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch from 927d72d to d6033e9 Compare October 26, 2021 19:25
@lidavidm lidavidm marked this pull request as ready for review October 26, 2021 19:25
@lidavidm lidavidm force-pushed the arrow-opentelemetry branch from a5d1b0f to d920399 Compare November 30, 2021 21:35
@lidavidm lidavidm closed this in bca0681 Dec 1, 2021
@lidavidm lidavidm deleted the arrow-opentelemetry branch December 1, 2021 19:34
@ursabot
Copy link

ursabot commented Dec 1, 2021

Benchmark runs are scheduled for baseline = 200f167 and contender = bca0681. bca0681 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️1.61% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.44% ⬆️0.04%] ursa-thinkcentre-m75q
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True

@pitrou
Copy link
Member

pitrou commented Dec 1, 2021

I'm curious: why are gRPC and protobuf required?

@lidavidm
Copy link
Member Author

lidavidm commented Dec 1, 2021

I'm curious: why are gRPC and protobuf required?

This PR configures the OTLP exporter so we can dump collected data somewhere where we can analyze it. The OTLP exporter talks to the OTLP collector over either HTTP or gRPC, using Protobuf payloads, so at least Protobuf and one of cURL and gRPC is required. We only enable the HTTP protocol, but the upstream CMake config currently doesn't differentiate between whether you have gRPC enabled or not when generating Protobuf stubs, so gRPC is unconditionally required. (See open-telemetry/opentelemetry-cpp#1045.) I think this may have just been fixed.

@pitrou
Copy link
Member

pitrou commented Dec 1, 2021

It would be nice to make gRPC at least optional, as it's a rather heavy dependency.

@lidavidm
Copy link
Member Author

lidavidm commented Dec 1, 2021

The fix open-telemetry/opentelemetry-cpp@eca856d is in v1.1.0 so we should be able to remove the gRPC dependency in ARROW-14957.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants