Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
39 changes: 39 additions & 0 deletions cmake/Findtelemetry.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#
# If not stated otherwise in this file or this component's LICENSE file the
# following copyright and licenses apply:
#
# Copyright 2026 Sky UK
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

if(NOT NATIVE_BUILD)
find_path(TELEMETRY_INCLUDE_DIR NAMES telemetry_busmessage_sender.h)
find_library(TELEMETRY_LIBRARY NAMES telemetry_msgsender)

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(TELEMETRY DEFAULT_MSG TELEMETRY_LIBRARY TELEMETRY_INCLUDE_DIR)

mark_as_advanced(TELEMETRY_INCLUDE_DIR TELEMETRY_LIBRARY)

if(TELEMETRY_FOUND)
set(TELEMETRY_LIBRARIES ${TELEMETRY_LIBRARY})
set(TELEMETRY_INCLUDE_DIRS ${TELEMETRY_INCLUDE_DIR})
else()
set(TELEMETRY_LIBRARIES "")
set(TELEMETRY_INCLUDE_DIRS "")
endif()
else()
set(TELEMETRY_INCLUDE_DIRS "")
set(TELEMETRY_LIBRARIES "")
endif()
45 changes: 38 additions & 7 deletions logging/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

set(RialtoLogging_HEADERS
include/RialtoLogging.h
include/RialtoTelemetry.h
)

set(RialtoLogging_SOURCES
Expand All @@ -34,20 +35,50 @@ set(RialtoLogging_INCLUDES
)

add_library(RialtoLogging STATIC ${RialtoLogging_HEADERS} ${RialtoLogging_SOURCES})
target_include_directories(RialtoLogging PUBLIC "$<BUILD_INTERFACE:${RialtoLogging_INCLUDES}>")
set_target_properties(RialtoLogging PROPERTIES CXX_STANDARD 17 CXX_STANDARD_REQUIRED ON CXX_EXTENSIONS OFF)
set_target_properties(RialtoLogging PROPERTIES POSITION_INDEPENDENT_CODE ON)
target_include_directories(RialtoLogging PUBLIC
"$<BUILD_INTERFACE:${RialtoLogging_INCLUDES}>"
)

# Telemetry support
if(CMAKE_TELEMETRY_2_0_REQUIRED)
message(STATUS "Telemetry 2.0 support enabled")
target_compile_definitions(RialtoLogging PUBLIC RIALTO_TELEMETRY_SUPPORT=1)
target_include_directories(RialtoLogging PUBLIC ${TELEMETRY_INCLUDE_DIRS})
target_link_libraries(RialtoLogging PUBLIC ${TELEMETRY_LIBRARIES})
Comment on lines +43 to +47
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

TELEMETRY_INCLUDE_DIRS / TELEMETRY_LIBRARIES are used here, but there is no find_package(telemetry) (or equivalent include of Findtelemetry.cmake) anywhere in the repo, so these variables will be undefined and telemetry linking will not work when CMAKE_TELEMETRY_2_0_REQUIRED is enabled. Please ensure the telemetry find-module is invoked before these target_* calls.

Copilot uses AI. Check for mistakes.
else()
message(STATUS "Telemetry support disabled (stub mode)")
endif()

set_target_properties(RialtoLogging PROPERTIES
CXX_STANDARD 17
CXX_STANDARD_REQUIRED ON
CXX_EXTENSIONS OFF
POSITION_INDEPENDENT_CODE ON
)

# EthanLog support
find_package(EthanLog)
if (EthanLog_FOUND AND RIALTO_ENABLE_ETHAN_LOG)
message(STATUS "EthanLog is enabled")
add_library(RialtoEthanLog STATIC ${RialtoLogging_HEADERS} ${RialtoLogging_SOURCES})
target_include_directories(RialtoEthanLog PUBLIC "$<BUILD_INTERFACE:${RialtoLogging_INCLUDES}>")
target_compile_definitions(RialtoEthanLog PRIVATE USE_ETHANLOG)
target_link_libraries(RialtoEthanLog PUBLIC EthanLog::EthanLog)
set_target_properties(RialtoEthanLog PROPERTIES CXX_STANDARD 17 CXX_STANDARD_REQUIRED ON CXX_EXTENSIONS OFF)
set_target_properties(RialtoEthanLog PROPERTIES POSITION_INDEPENDENT_CODE ON)
else ()
if(CMAKE_TELEMETRY_2_0_REQUIRED)
message(STATUS "Telemetry 2.0 support enabled")
target_compile_definitions(RialtoLogging PUBLIC RIALTO_TELEMETRY_SUPPORT=1)
target_include_directories(RialtoLogging PUBLIC ${TELEMETRY_INCLUDE_DIRS})
target_link_libraries(RialtoLogging PUBLIC ${TELEMETRY_LIBRARIES})
Comment on lines +69 to +71
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

In the EthanLog_FOUND branch, the telemetry configuration is duplicated and is applied to RialtoLogging (not RialtoEthanLog). This likely leaves RialtoEthanLog without RIALTO_TELEMETRY_SUPPORT/telemetry link deps and also duplicates status output; apply the telemetry target_* calls to RialtoEthanLog or factor the configuration into a shared helper.

Suggested change
target_compile_definitions(RialtoLogging PUBLIC RIALTO_TELEMETRY_SUPPORT=1)
target_include_directories(RialtoLogging PUBLIC ${TELEMETRY_INCLUDE_DIRS})
target_link_libraries(RialtoLogging PUBLIC ${TELEMETRY_LIBRARIES})
target_compile_definitions(RialtoEthanLog PUBLIC RIALTO_TELEMETRY_SUPPORT=1)
target_include_directories(RialtoEthanLog PUBLIC ${TELEMETRY_INCLUDE_DIRS})
target_link_libraries(RialtoEthanLog PUBLIC ${TELEMETRY_LIBRARIES})

Copilot uses AI. Check for mistakes.
else()
message(STATUS "Telemetry support disabled (stub mode)")
endif()
set_target_properties(RialtoEthanLog PROPERTIES
CXX_STANDARD 17
CXX_STANDARD_REQUIRED ON
CXX_EXTENSIONS OFF
POSITION_INDEPENDENT_CODE ON
)
else()
message(STATUS "EthanLog is disabled")
add_library(RialtoEthanLog ALIAS RialtoLogging)
endif ()
endif()
1 change: 1 addition & 0 deletions logging/include/RialtoLogging.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ extern "C"
{
#endif

#include "RialtoTelemetry.h"
#include <stdarg.h>
#include <stdint.h>

Expand Down
93 changes: 93 additions & 0 deletions logging/include/RialtoTelemetry.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* If not stated otherwise in this file or this component's LICENSE file the
* following copyright and licenses apply:
*
* Copyright 2026 Sky UK
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef RIALTO_TELEMETRY_H_
#define RIALTO_TELEMETRY_H_

#ifdef RIALTO_TELEMETRY_SUPPORT
#include <telemetry_busmessage_sender.h>
#endif

#ifdef __cplusplus
extern "C"
{
#endif

#ifdef RIALTO_TELEMETRY_SUPPORT

#define TELEMETRY_INIT(component) \
do \
{ \
t2_init(reinterpret_cast<char *>(component)); \
Comment on lines +34 to +37
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

TELEMETRY_INIT casts the component argument to char* using reinterpret_cast, which discards constness and can be undefined behavior if the telemetry library writes to the buffer. Prefer const_cast<char*> (if the API truly requires char*) or wrap t2_init behind a helper that accepts const char*.

Suggested change
#define TELEMETRY_INIT(component) \
do \
{ \
t2_init(reinterpret_cast<char *>(component)); \
static inline void rialtoTelemetryInit(const char *component)
{
t2_init(const_cast<char *>(component));
}
#define TELEMETRY_INIT(component) \
do \
{ \
rialtoTelemetryInit(component); \

Copilot uses AI. Check for mistakes.
} while (0)

#define TELEMETRY_UNINIT() \
do \
{ \
t2_uninit(); \
} while (0)

#define TELEMETRY_EVENT_STRING(marker, value) \
do \
{ \
t2_event_s(reinterpret_cast<char *>(marker), reinterpret_cast<char *>(value)); \
} while (0)
Comment on lines +46 to +50
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

TELEMETRY_EVENT_STRING casts marker/value to char* with reinterpret_cast, which discards constness and can be undefined behavior. Use const_cast<char*> at most, or update the wrapper to accept const char* so callers don’t need to worry about mutability.

Copilot uses AI. Check for mistakes.

#define TELEMETRY_EVENT_FLOAT(marker, value) \
do \
{ \
t2_event_f(reinterpret_cast<char *>(marker), static_cast<double>(value)); \
} while (0)

#define TELEMETRY_EVENT_INT(marker, value) \
do \
{ \
t2_event_d(reinterpret_cast<char *>(marker), static_cast<int>(value)); \
} while (0)

#else /* stub implementation if telemetry not enabled */

#define TELEMETRY_INIT(component) \
do \
{ \
} while (0)
#define TELEMETRY_UNINIT() \
do \
{ \
} while (0)
#define TELEMETRY_EVENT_STRING(m, v) \
do \
{ \
} while (0)
#define TELEMETRY_EVENT_FLOAT(m, v) \
do \
{ \
} while (0)
#define TELEMETRY_EVENT_INT(m, v) \
do \
{ \
} while (0)

#endif /* RIALTO_TELEMETRY_SUPPORT */

#ifdef __cplusplus
}
#endif

#endif /* RIALTO_TELEMETRY_H_ */
1 change: 1 addition & 0 deletions media/client/common/include/RialtoClientLogging.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#define RIALTO_CLIENT_LOGGING_H_

#include "RialtoLogging.h"
#include <cstdio>

#ifdef __cplusplus
extern "C"
Expand Down
18 changes: 18 additions & 0 deletions media/client/ipc/source/ControlIpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ bool ControlIpc::getSharedMemory(int32_t &fd, uint32_t &size)
if (ipcController->Failed())
{
RIALTO_CLIENT_LOG_ERROR("failed to get the shared memory due to '%s'", ipcController->ErrorText().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff), "Failed to get the shared memory due to '%s'",
ipcController->ErrorText().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - ControllIpc", telemetryBuff);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Telemetry marker string has a typo: "ControllIpc" (double 'l'). This makes telemetry harder to query/aggregate consistently; rename it to match the component/class name (e.g., "ControlIpc").

Suggested change
TELEMETRY_EVENT_STRING("Rialto Client - ControllIpc", telemetryBuff);
TELEMETRY_EVENT_STRING("Rialto Client - ControlIpc", telemetryBuff);

Copilot uses AI. Check for mistakes.
return false;
}

Expand Down Expand Up @@ -152,6 +156,10 @@ bool ControlIpc::registerClient()
if (ipcController->Failed())
{
RIALTO_CLIENT_LOG_ERROR("failed to register client due to '%s'", ipcController->ErrorText().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff), "Failed to register client due to '%s'",
ipcController->ErrorText().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - ControllIpc", telemetryBuff);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Telemetry marker string typo repeated here: "ControllIpc" should be "ControlIpc" to keep telemetry naming consistent.

Suggested change
TELEMETRY_EVENT_STRING("Rialto Client - ControllIpc", telemetryBuff);
TELEMETRY_EVENT_STRING("Rialto Client - ControlIpc", telemetryBuff);

Copilot uses AI. Check for mistakes.
return false;
}

Expand Down Expand Up @@ -181,6 +189,13 @@ bool ControlIpc::registerClient()
RIALTO_CLIENT_LOG_ERROR("Server and Client proto schema versions are not compatible. Server schema version: "
"%s, Client schema version: %s",
kServerSchemaVersion.str().c_str(), kCurrentSchemaVersion.str().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff),
"Server and Client proto schema versions are not compatible. Server schema version: "
"%s, Client schema version: %s",
kServerSchemaVersion.str().c_str(), kCurrentSchemaVersion.str().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - ControllIpc", telemetryBuff);
Comment on lines +193 to +197
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Telemetry marker string typo repeated here: "ControllIpc" should be "ControlIpc" to keep telemetry naming consistent.

Copilot uses AI. Check for mistakes.

return false;
}

Expand Down Expand Up @@ -260,6 +275,9 @@ void ControlIpc::onPing(const std::shared_ptr<firebolt::rialto::PingEvent> &even
if (ipcController->Failed())
{
RIALTO_CLIENT_LOG_ERROR("failed to ack due to '%s'", ipcController->ErrorText().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff), "Failed to ack due to '%s'", ipcController->ErrorText().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - ControllIpc", telemetryBuff);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Telemetry marker string typo repeated here: "ControllIpc" should be "ControlIpc" to keep telemetry naming consistent.

Suggested change
TELEMETRY_EVENT_STRING("Rialto Client - ControllIpc", telemetryBuff);
TELEMETRY_EVENT_STRING("Rialto Client - ControlIpc", telemetryBuff);

Copilot uses AI. Check for mistakes.
}
}
}; // namespace firebolt::rialto::client
4 changes: 4 additions & 0 deletions media/client/ipc/source/IpcClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ void IpcClient::processIpcThread()
if (!m_disconnecting)
{
RIALTO_CLIENT_LOG_ERROR("The ipc channel unexpectedly disconnected, destroying the channel");
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff),
"The ipc channel unexpectedly disconnected, destroying the channel");
TELEMETRY_EVENT_STRING("Rialto Client - IpcClient", telemetryBuff);

// Safe to destroy the ipc objects in the ipc thread as the client has already disconnected.
// This ensures the channel is destructed and that all ongoing ipc calls are unblocked.
Expand Down
24 changes: 24 additions & 0 deletions media/client/ipc/source/MediaKeysCapabilitiesIpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ std::shared_ptr<IMediaKeysCapabilitiesIpcFactory> IMediaKeysCapabilitiesIpcFacto
catch (const std::exception &e)
{
RIALTO_CLIENT_LOG_ERROR("Failed to create the media keys capabilities ipc factory, reason: %s", e.what());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff),
"Failed to create the media keys capabilities ipc factory, reason: %s", e.what());
TELEMETRY_EVENT_STRING("Rialto Client - MediaKeysCapabilitiesIpc", telemetryBuff);
}

return factory;
Expand All @@ -58,6 +62,10 @@ std::shared_ptr<IMediaKeysCapabilities> MediaKeysCapabilitiesIpcFactory::getMedi
catch (const std::exception &e)
{
RIALTO_CLIENT_LOG_ERROR("Failed to create the media keys capabilities ipc, reason: %s", e.what());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff),
"Failed to create the media keys capabilities ipc, reason: %s", e.what());
TELEMETRY_EVENT_STRING("Rialto Client - MediaKeysCapabilitiesIpc", telemetryBuff);
}

MediaKeysCapabilitiesIpcFactory::m_mediaKeysCapabilitiesIpc = mediaKeysCapabilitiesIpc;
Expand Down Expand Up @@ -115,6 +123,10 @@ std::vector<std::string> MediaKeysCapabilitiesIpc::getSupportedKeySystems()
if (ipcController->Failed())
{
RIALTO_CLIENT_LOG_ERROR("failed to get supported key systems due to '%s'", ipcController->ErrorText().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff), "Failed to get supported key systems due to '%s'",
ipcController->ErrorText().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - MediaKeysCapabilitiesIpc", telemetryBuff);
return {};
}

Expand Down Expand Up @@ -144,6 +156,10 @@ bool MediaKeysCapabilitiesIpc::supportsKeySystem(const std::string &keySystem)
if (ipcController->Failed())
{
RIALTO_CLIENT_LOG_ERROR("failed to supports key system due to '%s'", ipcController->ErrorText().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff), "Failed to support key systems due to '%s'",
ipcController->ErrorText().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - MediaKeysCapabilitiesIpc", telemetryBuff);
return false;
}

Expand Down Expand Up @@ -175,6 +191,10 @@ bool MediaKeysCapabilitiesIpc::getSupportedKeySystemVersion(const std::string &k
{
RIALTO_CLIENT_LOG_ERROR("failed to get supported key system version due to '%s'",
ipcController->ErrorText().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff), "Failed to get supported key system versions due to '%s'",
ipcController->ErrorText().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - MediaKeysCapabilitiesIpc", telemetryBuff);
return false;
}
version = response.version();
Expand Down Expand Up @@ -207,6 +227,10 @@ bool MediaKeysCapabilitiesIpc::isServerCertificateSupported(const std::string &k
{
RIALTO_CLIENT_LOG_ERROR("failed to check if server certificate is supported due to '%s'",
ipcController->ErrorText().c_str());
char telemetryBuff[128] = {0};
snprintf(telemetryBuff, sizeof(telemetryBuff), "Failed to check if server certificate is supported due to '%s'",
ipcController->ErrorText().c_str());
TELEMETRY_EVENT_STRING("Rialto Client - MediaKeysCapabilitiesIpc", telemetryBuff);
return false;
}

Expand Down
Loading
Loading