Build librdkafka on Windows #6095

flappy-sh · 2019-12-03T14:48:19Z

The PR will be reviewed by an osquery committer.
Here are some common things we look for:

The code is formatted correctly, considering using make format_check.
Common utilities within ./osquery/utils are used where appropriate (avoid reinventions).
Modern C++ structures and patterns are used whenever possible.
No memory or file descriptor leaks, please check all early-return and destructors.
No explicit casting, such as return (int)my_var, instead use static_cast.
The minimal amount of includes are used, only include what you use.
Comments for methods, structures, and classes follow our common patterns.
Status and LOG(N) messages do not use punctuation or contractions.
Support for both CMake and BUCK (we are happy to help).
The code mostly looks and feels similar to the existing codebase.

-->
Hi, this PR implements the #5678 . Let me know if there are more any additional changes required.

linux-foundation-easycla · 2019-12-03T14:48:22Z

The committers are authorized under a signed CLA.

✅ flappy-sh (92ca73f, 08b1b32, 943b6c5, 757995c, 9d417d2)

muffins

For the most part this lgtm, although I'm really not familiar with Kafka, so I'll leave accepting to someone a bit more well versed. I had a note about comments, but over lgtm. Thanks a lot for shipping this!

libraries/cmake/source/librdkafka/CMakeLists.txt

Smjert · 2019-12-04T14:33:42Z

Also please check the CI which is failing.

libraries/cmake/source/librdkafka/CMakeLists.txt

Smjert · 2019-12-05T13:01:52Z

@flappy-sh you can disregard my comment about xxhash, I've dealt with it here #6101

libraries/cmake/source/librdkafka/CMakeLists.txt

Smjert · 2019-12-13T02:13:09Z

Beware that you have probably mixed a merge with a rebase and now the PR it's including commits that were already merged in master.

flappy-sh · 2020-01-22T09:39:39Z

Hello @Smjert , sorry for the delay was on holiday. Regarding the merge and rebase , i am still testing,
Small question , why the azure pipelines use 2017 and the instruction for local build say 2019 ?

Smjert · 2020-01-22T12:51:13Z

Hello @Smjert , sorry for the delay was on holiday. Regarding the merge and rebase , i am still testing,
Small question , why the azure pipelines use 2017 and the instruction for local build say 2019 ?

@flappy-sh don't worry, happy to have you back!
We mainly kept the last good known Windows Server version tested, so Windows Server 2016.
On the CI to use VS2019 we would have to switch to Windows Server 2019.
While the instructions make you install VS2019, the toolchain used is v141, which is the same for VS2017.
The only thing that now changes is the SDK, which I noticed only now that there's a considerable mismatch of versions. I'll look into pinning a specific version.

flappy-sh · 2020-01-28T10:04:49Z

@Smjert okay , testing is almost complete . Now i have to address the merge rebase 'issue' and the headers are published correctly from libzstd as you requested! Let me know if there are more changes(i also have to remove the testing comments)

plugins/logger/CMakeLists.txt

libraries/cmake/source/librdkafka/CMakeLists.txt

includes old commits like: added comments, updated comments , fixed some whitespace , added submodule , fixed comments , pipeline upgrades 'espase mia solina sto ergostasio' , z std fix try , kafka windows v1.0 golden s.p , osquery with kafka on windows initial commit. builds , added submodule , kafka windows test 'xronia polla spk' , include system , cmake added kafka producer source in windows 'psaxnontas ton mpetoven' , relative path handling , testing cmake pipeline , z std compression fix

flappy-sh · 2020-03-09T09:40:21Z

@theopolis @Smjert @alessandrogario @muffins Thank you for the merge ! If you need anything else just ping me.

skydmh · 2020-04-21T02:44:53Z

@flappy-sh
Hello
Nice to meet you
Could you tell me when kafka_prodecer on windows will be released?
thank you very much

flappy-sh · 2020-04-22T09:20:17Z

@flappy-sh
Hello
Nice to meet you
Could you tell me when kafka_prodecer on windows will be released?
thank you very much

Hello @skydmh , this changes were merged to master , you can build the branch and have it available, otherwise you can wait for an official release containing the changes.

Smjert · 2020-04-22T09:27:32Z

@flappy-sh, @skydmh unfortunately the kafka producer is not actually yet available.
The rdkafka library is indeed built but it's not linked/used with the producer, especially since the kafka producer CMake target is just a fake library on Windows.

@flappy-sh: You can see this here

osquery/plugins/logger/CMakeLists.txt

Lines 148 to 171 in 9e116e5

    
           if(DEFINED PLATFORM_LINUX OR DEFINED PLATFORM_MACOS) 
        
             add_osquery_library(plugins_logger_kafkaproducer EXCLUDE_FROM_ALL 
        
               kafka_producer.cpp 
        
             ) 
        
             enableLinkWholeArchive(plugins_logger_kafkaproducer) 
        
             target_link_libraries(plugins_logger_kafkaproducer PUBLIC 
        
               osquery_cxx_settings 
        
               plugins_logger_commondeps 
        
               osquery_dispatcher 
        
               osquery_utils_config 
        
               thirdparty_librdkafka 
        
             ) 
        
             set(public_header_files 
        
               kafka_producer.h 
        
             ) 
        
             generateIncludeNamespace(plugins_logger_kafkaproducer "plugins/logger" "FILE_ONLY" ${public_header_files}) 
        
             add_test(NAME plugins_logger_tests_kafkaproducerloggertests-test COMMAND plugins_logger_tests_kafkaproducerloggertests-test) 
        
           else() 
        
             add_osquery_library(plugins_logger_kafkaproducer INTERFACE) 
        
           endif()

The real target is only created if it's Linux or macOS. Yesterday I quickly tried removing that if, but there are link errors with missing symbols.

The relative issue is still opened here #5678

skydmh · 2020-04-23T09:37:20Z

@flappy-sh
hello
any update for Smjert's comment?
thanks a lot

flappy-sh · 2020-04-23T09:51:44Z

Hi @skydmh , as as mentioned before please take a look at the code otherwise you can wait for an official release containing the changes.

skydmh · 2020-04-23T10:00:27Z

@flappy-sh
Thank you very much for your kind reply.

Could you let me know the estimated time of the official release containing the changes

Smjert · 2020-04-23T10:09:37Z

@flappy-sh I'm sorry if I wasn't clear, this PR is present in the latest osquery release, 4.3.0, but it doesn't implement the kafka producer for Windows.
As I was saying here #6095 (comment), the librdkafka library is not actually used/linked on a Windows platform and the kafka producer itself, as it can be seen in the CMake code, is not even compiled on Windows.

flappy-sh · 2020-04-29T09:51:58Z

@Smjert About that , my mindmap of dependency's to enable it is a little hazy , but when i pushed that branch at that point of time it talked and shipped data (windows server 2016 cpu query's) to a kafka cluster (running on linux farm) so from my point of view the branch was ready (especially after the linker collision patch), for the fact why now it doesn't compile i really cant say . I only coded it ...

muffins added Windows feature labels Dec 4, 2019

muffins reviewed Dec 4, 2019

View reviewed changes

libraries/cmake/source/librdkafka/CMakeLists.txt Outdated Show resolved Hide resolved

Smjert requested changes Dec 4, 2019

View reviewed changes

Smjert reviewed Dec 4, 2019

View reviewed changes

libraries/cmake/source/librdkafka/CMakeLists.txt Outdated Show resolved Hide resolved

Smjert changed the title ~~Kafka windows~~ Build librdkafka on Windows Dec 4, 2019

Smjert reviewed Dec 12, 2019

View reviewed changes

libraries/cmake/source/librdkafka/CMakeLists.txt Outdated Show resolved Hide resolved

flappy-sh commented Dec 13, 2019

View reviewed changes

libraries/cmake/source/librdkafka/CMakeLists.txt Show resolved Hide resolved

flappy-sh force-pushed the kafka_windows branch from 4b2c2b7 to e756aed Compare January 28, 2020 14:39

flappy-sh requested a review from Smjert January 28, 2020 15:45

alessandrogario reviewed Feb 20, 2020

View reviewed changes

plugins/logger/CMakeLists.txt Outdated Show resolved Hide resolved

alessandrogario reviewed Feb 20, 2020

View reviewed changes

libraries/cmake/source/librdkafka/CMakeLists.txt Outdated Show resolved Hide resolved

alessandrogario reviewed Feb 20, 2020

View reviewed changes

libraries/cmake/source/librdkafka/CMakeLists.txt Outdated Show resolved Hide resolved

flappy-sh added 2 commits February 25, 2020 16:14

removed comments, and some platform checks

bf10513

flappy-sh force-pushed the kafka_windows branch from 3bb5ef5 to bf10513 Compare February 25, 2020 15:15

flappy-sh and others added 3 commits February 25, 2020 20:16

CI test

99c6d0c

CI test

bbc4b48

Update CMakeLists.txt to remove tabs

15552f1

Smjert approved these changes Mar 6, 2020

View reviewed changes

theopolis merged commit d9faba2 into osquery:master Mar 6, 2020

flappy-sh deleted the kafka_windows branch April 23, 2020 09:52

herbmaster mentioned this pull request May 20, 2020

Link librdkafka on windows #6454

Merged

mike-myers-tob mentioned this pull request Sep 23, 2021

Kafka & SSL support for osquery on Windows #5678

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build librdkafka on Windows #6095

Build librdkafka on Windows #6095

flappy-sh commented Dec 3, 2019

linux-foundation-easycla bot commented Dec 3, 2019 •

edited

muffins left a comment

Smjert commented Dec 4, 2019

Smjert commented Dec 5, 2019

Smjert commented Dec 13, 2019

flappy-sh commented Jan 22, 2020

Smjert commented Jan 22, 2020 •

edited

flappy-sh commented Jan 28, 2020

flappy-sh commented Mar 9, 2020

skydmh commented Apr 21, 2020

flappy-sh commented Apr 22, 2020

Smjert commented Apr 22, 2020 •

edited

skydmh commented Apr 23, 2020

flappy-sh commented Apr 23, 2020

skydmh commented Apr 23, 2020

Smjert commented Apr 23, 2020 •

edited

flappy-sh commented Apr 29, 2020

Build librdkafka on Windows #6095

Build librdkafka on Windows #6095

Conversation

flappy-sh commented Dec 3, 2019

linux-foundation-easycla bot commented Dec 3, 2019 • edited

muffins left a comment

Choose a reason for hiding this comment

Smjert commented Dec 4, 2019

Smjert commented Dec 5, 2019

Smjert commented Dec 13, 2019

flappy-sh commented Jan 22, 2020

Smjert commented Jan 22, 2020 • edited

flappy-sh commented Jan 28, 2020

flappy-sh commented Mar 9, 2020

skydmh commented Apr 21, 2020

flappy-sh commented Apr 22, 2020

Smjert commented Apr 22, 2020 • edited

skydmh commented Apr 23, 2020

flappy-sh commented Apr 23, 2020

skydmh commented Apr 23, 2020

Smjert commented Apr 23, 2020 • edited

flappy-sh commented Apr 29, 2020

linux-foundation-easycla bot commented Dec 3, 2019 •

edited

Smjert commented Jan 22, 2020 •

edited

Smjert commented Apr 22, 2020 •

edited

Smjert commented Apr 23, 2020 •

edited