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.
Comment on lines +4 to +8
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

Findtelemetry.cmake uses find_package_handle_standard_args(TELEMETRY ...) which sets TELEMETRY_FOUND, but the project calls find_package(telemetry ...), which expects telemetry_FOUND. This case mismatch can cause find_package(telemetry REQUIRED) to fail even when the library is present. Use find_package_handle_standard_args(telemetry ...) (or otherwise set telemetry_FOUND) to match the package name.

Copilot uses AI. Check for mistakes.
# 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)
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

find_package_handle_standard_args(TELEMETRY ...) uses an uppercase package name that won’t set the telemetry_FOUND variable expected by find_package(telemetry ...) (CMake variables are case-sensitive). This can cause find_package(telemetry REQUIRED) to fail even when the library/header are found. Use find_package_handle_standard_args(telemetry ...) (matching the find_package call) and set the TELEMETRY_* output variables accordingly.

Suggested change
find_package_handle_standard_args(TELEMETRY DEFAULT_MSG TELEMETRY_LIBRARY TELEMETRY_INCLUDE_DIR)
find_package_handle_standard_args(telemetry DEFAULT_MSG TELEMETRY_LIBRARY TELEMETRY_INCLUDE_DIR)
set(TELEMETRY_FOUND ${telemetry_FOUND})

Copilot uses AI. Check for mistakes.

Comment on lines +20 to +26
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

Findtelemetry.cmake calls find_package_handle_standard_args with package name "TELEMETRY" (line 25). Since the project uses find_package(telemetry ...), this will set TELEMETRY_FOUND instead of telemetry_FOUND, so find_package(telemetry REQUIRED) may incorrectly fail even when the library is present. Also, when NATIVE_BUILD is true, no *_FOUND variable is set at all, so REQUIRED will always fail. Use the same package name casing as find_package() (e.g., telemetry) in find_package_handle_standard_args, and ensure the module sets telemetry_FOUND appropriately (or avoid REQUIRED unless telemetry is enabled).

Copilot uses AI. Check for mistakes.
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()
Comment on lines +20 to +35
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

The module filename suggests it will be used via find_package(telemetry), but find_package_handle_standard_args is invoked with TELEMETRY, producing TELEMETRY_FOUND rather than telemetry_FOUND. This case mismatch can make find_package(telemetry) report "not found" even when the library/header are present. Align the package name/casing throughout (filename, find_package_handle_standard_args argument, and _FOUND variable checks).

Copilot uses AI. Check for mistakes.
else()
set(TELEMETRY_INCLUDE_DIRS "")
set(TELEMETRY_LIBRARIES "")
endif()
48 changes: 41 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 @@ -27,27 +28,60 @@ set(RialtoLogging_SOURCES
source/LogFileHandle.cpp
source/LogFileHandle.h
source/RialtoLogging.cpp
source/RialtoTelemetry.cpp
)

set(RialtoLogging_INCLUDES
"${CMAKE_CURRENT_SOURCE_DIR}/include"
)

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")
find_package(telemetry REQUIRED)
add_definitions( -DRIALTO_TELEMETRY_SUPPORT=1 )
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 +50
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

Telemetry support is enabled based on CMAKE_TELEMETRY_2_0_REQUIRED, but this CMakeLists never calls find_package(telemetry ...) (or otherwise defines TELEMETRY_INCLUDE_DIRS / TELEMETRY_LIBRARIES). As written, turning telemetry on can break the build because <telemetry_busmessage_sender.h> won’t be found and the link libraries may be empty. Consider invoking the new Findtelemetry module when telemetry is enabled and failing early if the dependency is required but not found.

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(RialtoEthanLog PUBLIC RIALTO_TELEMETRY_SUPPORT=1)
target_include_directories(RialtoEthanLog PUBLIC ${TELEMETRY_INCLUDE_DIRS})
target_link_libraries(RialtoEthanLog PUBLIC ${TELEMETRY_LIBRARIES})
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
54 changes: 54 additions & 0 deletions logging/include/RialtoTelemetry.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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

void TELEMETRY_INIT(const char* component);
void TELEMETRY_UNINIT();
void TELEMETRY_EVENT_STRING(const char* marker, const char* value);
void TELEMETRY_EVENT_FLOAT(const char* marker, float value);
void TELEMETRY_EVENT_INT(const char* marker, int value);

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

inline void TELEMETRY_INIT(const char* component) {}
inline void TELEMETRY_UNINIT() {}
inline void TELEMETRY_EVENT_STRING(const char* marker, const char* value) {}
inline void TELEMETRY_EVENT_FLOAT(const char* marker, float value) {}
inline void TELEMETRY_EVENT_INT(const char* marker, int value) {}

#endif /* RIALTO_TELEMETRY_SUPPORT */

#ifdef __cplusplus
}
#endif

#endif /* RIALTO_TELEMETRY_H_ */
71 changes: 71 additions & 0 deletions logging/source/RialtoTelemetry.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* 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.
*/

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

#ifdef __cplusplus
extern "C"
{
#endif

#define RIALTO_TELEMETRY_LOG_MIL(fmt, args...) RIALTO_LOG_MIL(RIALTO_COMPONENT_SERVER, fmt, ##args)

#ifdef __cplusplus
}
#endif
Comment on lines +24 to +33

#ifdef RIALTO_TELEMETRY_SUPPORT

void TELEMETRY_INIT(const char* component)
{
RIALTO_TELEMETRY_LOG_MIL("Telemetry initialized for: %s", component);
printf("Telemetry initialized for: %s", component);
// Copy into a mutable buffer: t2_init takes char* and may write to it,
Comment on lines +37 to +41
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

RialtoTelemetry.cpp includes only RialtoLogging.h, so RIALTO_SERVER_LOG_MIL is not defined here (it’s declared in media/server/common/include/RialtoServerLogging.h). This will fail to compile; use the generic logging macro (e.g., RIALTO_LOG_MIL with an appropriate RIALTO_COMPONENT) or remove the log from this low-level wrapper.

Copilot uses AI. Check for mistakes.
// so passing a const/string-literal directly is undefined behavior.
char buf[64];
strncpy(buf, component, sizeof(buf) - 1);
buf[sizeof(buf) - 1] = '\0';
t2_init(buf);
Comment on lines +43 to +46
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

strncpy is used here but this file doesn’t include <cstring>/<string.h>, which will cause a compile error in C++. Add the proper header (and consider std::strncpy if you include <cstring>).

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +46
}
Comment on lines +20 to +47
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

This file uses RIALTO_SERVER_LOG_MIL, but it only includes RialtoLogging.h (which does not define the RIALTO_SERVER_LOG_* macros). This will not compile; either include RialtoServerLogging.h or log via the generic RIALTO_LOG_MIL(RIALTO_COMPONENT_..., ...) API so the telemetry wrapper can be used from both client and server builds.

Copilot uses AI. Check for mistakes.

void TELEMETRY_UNINIT()
{
t2_uninit();
}

void TELEMETRY_EVENT_STRING(const char* marker, const char* value)
{
RIALTO_LOG_MIL(RIALTO_COMPONENT_SERVER, "Telemetry marker: %s, value: %s", marker, value);
printf("Telemetry marker: %s, value: %s", marker, value);
t2_event_s(const_cast<char*>(marker), const_cast<char*>(value));
Comment on lines +24 to +58
}
Comment on lines +54 to +59
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

t2_event_s takes mutable char* arguments; const_casting marker/value is undefined behavior if the telemetry implementation writes into these buffers. Prefer copying into local mutable buffers (similar to what’s done for component in TELEMETRY_INIT) before calling t2_event_s.

Copilot uses AI. Check for mistakes.

void TELEMETRY_EVENT_FLOAT(const char* marker, float value)
{
t2_event_f(const_cast<char*>(marker), static_cast<double>(value));
}
Comment on lines +54 to +64
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

TELEMETRY_EVENT_* wrappers use const_cast<char*> to pass marker/value into the t2_event_* APIs. If those functions write to the buffers (the code already notes t2_init may), this becomes undefined behavior for string literals and other const storage. Prefer copying into mutable buffers (similar to TELEMETRY_INIT) or updating the wrapper to manage mutable storage internally.

Copilot uses AI. Check for mistakes.

void TELEMETRY_EVENT_INT(const char* marker, int value)
{
t2_event_d(const_cast<char*>(marker), static_cast<int>(value));
}

#endif /* RIALTO_TELEMETRY_SUPPORT */
Comment on lines +1 to +71
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"
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

Client code now uses TELEMETRY_* macros widely via files that include RialtoClientLogging.h, but this header doesn't include RialtoTelemetry.h. This will cause build failures (undefined TELEMETRY_EVENT_STRING, etc.) unless each .cpp includes the telemetry header separately. Consider including RialtoTelemetry.h here (or in RialtoLogging.h) so telemetry macros are available anywhere client logging is used.

Suggested change
#include "RialtoLogging.h"
#include "RialtoLogging.h"
#include "RialtoTelemetry.h"

Copilot uses AI. Check for mistakes.
#include <cstdio>
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

RialtoClientLogging.h now includes , but this header doesn’t use stdio APIs directly. Pulling into a widely-included logging header increases coupling/compile time; prefer including only in the .cpp files that call snprintf.

Suggested change
#include <cstdio>

Copilot uses AI. Check for mistakes.

#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 6, 2026

Choose a reason for hiding this comment

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

The telemetry marker string has a spelling mismatch ("Rialto Client - ControllIpc"). If telemetry dashboards/filters depend on stable marker names, this typo will fragment metrics. Please correct the marker to "Rialto Client - ControlIpc" (and keep it consistent across all events from this module).

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

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 6, 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 will fragment metrics under an incorrect marker name; please correct the marker identifier.

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 6, 2026

Choose a reason for hiding this comment

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

Telemetry marker name has a typo ("ControllIpc"). Please rename it to "ControlIpc" to keep markers consistent across the codebase/metrics.

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

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Same telemetry marker typo as above (ControllIpc -> ControlIpc).

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

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The telemetry marker string has a typo: "ControllIpc" (double “l”). This makes the marker inconsistent and harder to query/aggregate. Rename it to "ControlIpc" (and keep it consistent across all occurrences in this file).

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 +192 to +198
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

Telemetry marker name has a typo ("ControllIpc"). Please rename it to "ControlIpc" to keep markers consistent across the codebase/metrics.

Copilot uses AI. Check for mistakes.
Comment on lines +197 to +198
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Same telemetry marker typo as above (ControllIpc -> ControlIpc).

Copilot uses AI. Check for mistakes.
Comment on lines +192 to +198
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The telemetry marker string has a typo: "ControllIpc" (double “l”). This makes the marker inconsistent and harder to query/aggregate. Rename it to "ControlIpc" (and keep it consistent across all occurrences in this file).

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 6, 2026

Choose a reason for hiding this comment

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

Telemetry marker name has a typo ("ControllIpc"). Please rename it to "ControlIpc" to keep markers consistent across the codebase/metrics.

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

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Same telemetry marker typo as above (ControllIpc -> ControlIpc).

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

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The telemetry marker string has a typo: "ControllIpc" (double “l”). This makes the marker inconsistent and harder to query/aggregate. Rename it to "ControlIpc" (and keep it consistent across all occurrences in this file).

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