[Linux] Trying to compile static library against a small sample project gives a linker error

[lucianopa-msft]

Describe your environment.
Trying to build this library for linux x86_64 using the build.sh to generate a static library on WSL linux environment 24.04 and our code which is relatively small fails to link with the following error:

/home/lucianopa/project/src/cpp/wrapper.cpp:42:(.text._ZN21ExternalTelemetryHook13InspectRecordERN10CsProtocol6RecordE[_ZN21ExternalTelemetryHook13InspectRecordERN10CsProtocol6RecordE]+0x53): undefined reference to `Microsoft::Applications::Events::exporters::DecodeRecord(CsProtocol::Record const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)'
          collect2: error: ld returned 1 exit status

Steps to reproduce.
Build the library with the following command

./build.sh clean x86_64 noroot release -l static -D BUILD_STATIC_SQLITE=ON -D BUILD_FUNC_TESTS=OFF -D BUILD_UNIT_TESTS=OFF

Take the libMat.a artifact in out dir and try to reference Microsoft::Applications::Events::exporters::DecodeRecord(record, serializedEvent); on your code.

What is the expected behavior?
Compile and link successfully

What is the actual behavior?
Linker failed find symbol in the .a library with error:

/home/lucianopa/project/src/cpp/wrapper.cpp:42:(.text._ZN21ExternalTelemetryHook13InspectRecordERN10CsProtocol6RecordE[_ZN21ExternalTelemetryHook13InspectRecordERN10CsProtocol6RecordE]+0x53): undefined reference to `Microsoft::Applications::Events::exporters::DecodeRecord(CsProtocol::Record const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)'
          collect2: error: ld returned 1 exit status

Additional context.
We do the same for Windows (same codebase) which we compile as well and get the ClientTelemetry.lib and it works file.
Windows compilation command that we use: .\build-win.ps1 -archs x64 -configs Release -enableTests false -enableMini false -enableWin10 false -binTypes lib so artifacts should be the same.

Also I was able to generate a libmat.a that compiles and links successfully by adding PayloadDecoder.cpp to CMakeLists.txt source entry

cpp_client_telemetry/lib/CMakeLists.txt

Line 54 in 9db2568

pal/WorkerThread.cpp

Which for windows this seems to be automatically added by the vsxitems file.
So my question is if is that expected not to be compiled for linux because the PayloadDecoder.hpp is on the public include file so people are expected to use that symbol, so it didn't made much sense to me that I didn't see any reference to this file other than test folders in CMakeLists.txt files.
Am I using the correct build command? Should I use some other flags for it? Or this api shouldn't be public?

[lalitb]

It's a valid issue - Ideally I don't think the PayloadDecoder should be part of public API. The API is meant to be internal testing, and validation. The header should be removed from public directory, and also Windows build shouldn't expose it.

[lucianopa-msft]

Interesting, but we do use it in our code (maybe in the wrong way) so removing it would be a breaking change. I think for us it doesn't matter because we could fix it to use the correct way once we understand how to. But for others it may be breaking change.
We do use it for defining a string based hook for the record, so as long as we are able to serialize the record without it, it should be fine:
Sample

  // std::function<void(const char* event) event_hook;
  virtual bool InspectRecord(::CsProtocol::Record& record) noexcept override {
    if (event_hook) {
      std::string serializedEvent;
      Microsoft::Applications::Events::exporters::DecodeRecord(record,
                                                               serializedEvent);
      event_hook(serializedEvent.c_str());
    }
    return true;
  }

[lucianopa-msft]

@lalitb Just to give a little bit more context, serialization happens because we do pass this serialized event across an FFI boundary. So question is if there is an official publicly supported way to serialize a record?

[lalitb]

Got it, I think this is a legitimate use case since IDataInspector is a public interface that naturally requires record serialization. We should be adding decoder/PayloadDecoder.cpp in lib/CMakeLists.txt."

@lucianopa-msft Would you like to contribute the fix?

ps. - Please note that CsProtocol::Record is auto-generated and doesn't provide ABI stability and may change between releases, so ensure you continue to compile from source rather than dynamically linking against prebuilt libraries to avoid memory layout mismatches.

[lucianopa-msft]

@lucianopa-msft Would you like to contribute the fix?

Sure, be happy to :)

ps. - Please note that CsProtocol::Record is auto-generated and doesn't provide ABI stability and may change between releases, so ensure you continue to compile from source rather than dynamically linking against prebuilt libraries to avoid memory layout mismatches.

Right now we pin versions and we build of release tags, we cannot build completely from source because it would make our build workflow very complex. But given we pin the binary to a version and copy the headers from that version as well as update it periodically it should be fine right?