Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add session_id to endpoint logs #638

Merged
merged 2 commits into from
Aug 6, 2024
Merged

Conversation

Danielius1922
Copy link
Member

No description provided.

@ocf-conformance-test-tool
Copy link

🎉 Thank you for your code contribution! To guarantee the change/addition is conformant to the OCF Specification, we would like to ask you to execute OCF Conformance Testing of your change ☝️ when your work is ready to be reviewed.


ℹ️ To verify your latest change (49a12e5), label this PR with OCF Conformance Testing.

⚠️ Label is removed with every code change.

Copy link

coderabbitai bot commented Jul 29, 2024

Walkthrough

The recent updates significantly enhance logging capabilities across multiple modules, focusing on improving traceability for session and endpoint management. Key modifications include the introduction of new logging functions and the integration of session IDs into log messages, streamlining debugging processes and providing clearer insights into operations while preserving existing functionality.

Changes

Files Change Summary
api/oc_client_api.c, api/oc_session_events.c, api/oc_network_events.c Improved logging for session management and network events, adding new log statements and refining header inclusions for better structure.
api/oc_endpoint.c, api/oc_endpoint_internal.h, api/cloud/oc_cloud_internal.h Introduced oc_endpoint_log function to enhance endpoint logging with session ID details; new cloud endpoint logging function added.
port/linux/tcpsession.c Enhanced TCP session logging by including session IDs in debug statements and refactoring session ID assignment for consistency across functions.
api/cloud/oc_cloud.c, api/cloud/oc_cloud_manager.c Streamlined logging in cloud manager functions using oc_cloud_endpoint_log for improved clarity and consistency in logging actions.
security/oc_tls.c Updated logging in TLS peer management functions to include session IDs, enhancing traceability for operations related to managing peers.
util/oc_etimer.c Changed logging macros from OC_DBG to OC_TRACE, reflecting a shift in the logging strategy for timer management functions.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 122d8f5 and 49a12e5.

Files selected for processing (7)
  • api/oc_client_api.c (4 hunks)
  • api/oc_endpoint.c (1 hunks)
  • api/oc_endpoint_internal.h (2 hunks)
  • api/oc_network_events.c (1 hunks)
  • api/oc_session_events.c (4 hunks)
  • port/linux/tcpsession.c (10 hunks)
  • security/oc_tls.c (4 hunks)
Additional comments not posted (24)
api/oc_endpoint_internal.h (4)

23-23: Inclusion of oc_helpers.h looks good.

This inclusion likely provides additional utility functions.


24-24: Inclusion of port/oc_log_internal.h looks good.

This inclusion likely enhances logging capabilities.


28-28: Inclusion of inttypes.h looks good.

This inclusion improves portability and type safety across different platforms.


130-131: Declaration of oc_endpoint_short_log looks good.

This function enhances logging capabilities by logging session information. The OC_NONNULL annotation ensures robustness.

api/oc_network_events.c (1)

208-213: Enhanced logging in oc_network_drop_receive_events looks good.

Including the session ID and a descriptive endpoint format improves traceability and debugging capabilities.

api/oc_session_events.c (5)

19-19: Inclusion of util/oc_features.h looks good.

This inclusion replaces oc_config.h, indicating a shift in dependency management.


21-21: Inclusion of api/oc_endpoint_internal.h looks good.

This inclusion suggests that internal endpoint functionalities are now utilized within this file.


139-139: Enhanced logging in oc_session_start_event looks good.

The new logging statement improves traceability of session management by providing context about the specific sessions being started.


162-162: Enhanced logging in oc_session_end_event looks good.

The new logging statement improves traceability of session management by providing context about the specific sessions being ended.


356-357: Enhanced logging in oc_handle_session looks good.

The new logging statement improves debugging capabilities by providing clearer insights into session management processes.

api/oc_endpoint.c (1)

809-825: LGTM! Ensure proper usage of the new function.

The function oc_endpoint_short_log is well-implemented and follows best practices. It conditionally logs endpoint details based on the OC_DBG_IS_ENABLED flag.

api/oc_client_api.c (1)

964-964: LGTM! Ensure proper usage of the new logging function.

The addition of the oc_endpoint_short_log call in oc_close_session enhances logging and traceability of session details.

port/linux/tcpsession.c (8)

256-258: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of session creation.


514-515: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of session reception.


627-629: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of waiting session creation.


749-750: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of asynchronous session freeing.


765-766: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of asynchronous waiting session freeing.


1236-1237: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of waiting session retries.


1321-1322: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of waiting session processing.


1350-1351: LGTM! The added debug log message enhances traceability.

The inclusion of the session_id in the debug log message improves the traceability of waiting session processing.

security/oc_tls.c (4)

436-443: LGTM! Verify the function usage in the codebase.

The changes to include session ID in the log messages are approved.

However, ensure that all function calls to tls_process_drop_event_for_removed_endpoint are verified.


417-424: LGTM! Verify the function usage in the codebase.

The changes to include session ID in the log messages are approved.

However, ensure that all function calls to tls_process_drop_event_for_removed_endpoint are verified.

Verification successful

Verification successful.

The function tls_process_drop_event_for_removed_endpoint is correctly utilized in the codebase, and the changes to include session ID in the log messages are properly integrated.

  • security/oc_tls.c: Verified function calls to tls_process_drop_event_for_removed_endpoint.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `tls_process_drop_event_for_removed_endpoint`.

# Test: Search for the function usage. Expect: Only occurrences of the updated log messages with session ID.
rg --type c -A 5 $'tls_process_drop_event_for_removed_endpoint'

Length of output: 1328


478-482: LGTM! Verify the function usage in the codebase.

The changes to include session ID in the log messages are approved.

However, ensure that all function calls to oc_tls_free_peer are verified.

Verification successful

Function usage verified.

The function oc_tls_free_peer is used correctly throughout the codebase, and the log messages have been updated to include the session ID as expected.

  • security/oc_tls.c: Multiple instances of oc_tls_free_peer calls with updated log messages.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `oc_tls_free_peer`.

# Test: Search for the function usage. Expect: Only occurrences of the updated log messages with session ID.
rg --type c -A 5 $'oc_tls_free_peer'

Length of output: 5675


2202-2208: LGTM! Verify the function usage in the codebase.

The changes to include session ID in the log messages are approved.

However, ensure that all function calls to oc_tls_add_new_peer are verified.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 49a12e5 and d91c3e9.

Files selected for processing (8)
  • api/cloud/oc_cloud.c (2 hunks)
  • api/cloud/oc_cloud_deregister.c (1 hunks)
  • api/cloud/oc_cloud_internal.h (1 hunks)
  • api/cloud/oc_cloud_manager.c (3 hunks)
  • api/oc_client_api.c (4 hunks)
  • api/oc_endpoint.c (2 hunks)
  • api/oc_endpoint_internal.h (2 hunks)
  • api/oc_session_events.c (4 hunks)
Files skipped from review due to trivial changes (1)
  • api/cloud/oc_cloud_manager.c
Files skipped from review as they are similar to previous changes (3)
  • api/oc_client_api.c
  • api/oc_endpoint_internal.h
  • api/oc_session_events.c
Additional comments not posted (6)
api/cloud/oc_cloud_internal.h (1)

131-131: LGTM!

The function declaration for oc_cloud_endpoint_log looks good. The use of OC_NONNULL is appropriate to ensure non-null parameters.

api/cloud/oc_cloud_deregister.c (1)

190-190: LGTM!

The call to oc_cloud_endpoint_log simplifies the logging mechanism during deregistration. The integration looks correct.

api/cloud/oc_cloud.c (2)

131-151: LGTM!

The implementation of oc_cloud_endpoint_log looks correct. It conditionally logs the endpoint and session ID based on the enabled logging levels.


339-353: LGTM!

The additional logging in cloud_ep_session_event_handler improves traceability during session handling. The integration of the logging function looks correct.

api/oc_endpoint.c (2)

736-737: LGTM! The modification to ignore the next pointer is appropriate.

This change ensures that the next pointer is not considered when checking if the endpoint is empty, which aligns with the intended functionality.


811-827: LGTM! The new function oc_endpoint_log is well-implemented.

The function effectively uses conditional compilation to control logging behavior and includes relevant endpoint details in the log message.

However, ensure the correctness of the oc_endpoint_to_string64 function used within this function.

Verification successful

LGTM! The new function oc_endpoint_log is well-implemented.

The function effectively uses conditional compilation to control logging behavior and includes relevant endpoint details in the log message. The oc_endpoint_to_string64 function used within this function is correctly implemented.

  • oc_endpoint_to_string64 correctly handles null pointers, initializes the string, converts the endpoint to a C-string, and returns a boolean indicating success or failure.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of `oc_endpoint_to_string64`.

# Test: Search for the function implementation. Expect: Correct implementation.
rg --type-add 'c:*.c' --type c --pcre2 --multiline --multiline-dotall --context 5 --regexp 'bool\s+oc_endpoint_to_string64\s*\(.*\)\s*\{'

Length of output: 27737

@Danielius1922 Danielius1922 force-pushed the adam/feature/session_id_logs branch from d91c3e9 to 7d64e76 Compare July 29, 2024 17:53
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d91c3e9 and 7d64e76.

Files selected for processing (8)
  • api/cloud/oc_cloud.c (2 hunks)
  • api/cloud/oc_cloud_deregister.c (1 hunks)
  • api/cloud/oc_cloud_internal.h (1 hunks)
  • api/cloud/oc_cloud_manager.c (3 hunks)
  • api/oc_client_api.c (4 hunks)
  • api/oc_endpoint.c (2 hunks)
  • api/oc_endpoint_internal.h (2 hunks)
  • api/oc_session_events.c (4 hunks)
Files skipped from review as they are similar to previous changes (7)
  • api/cloud/oc_cloud.c
  • api/cloud/oc_cloud_deregister.c
  • api/cloud/oc_cloud_internal.h
  • api/cloud/oc_cloud_manager.c
  • api/oc_endpoint.c
  • api/oc_endpoint_internal.h
  • api/oc_session_events.c
Additional comments not posted (1)
api/oc_client_api.c (1)

964-964: LGTM! Verify the implementation of oc_endpoint_log.

The addition of the logging statement enhances traceability for session closures.

Verify the correct implementation of oc_endpoint_log to ensure it handles the endpoint parameter correctly.

Verification successful

LGTM! The implementation of oc_endpoint_log is correct.

The addition of the logging statement enhances traceability for session closures.

  • oc_endpoint_log is defined in api/oc_endpoint_internal.h.
  • oc_endpoint_log is implemented in api/oc_endpoint.c and correctly handles the endpoint parameter.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of `oc_endpoint_log`.

# Test: Search for the implementation of `oc_endpoint_log`. Expect: Correct handling of the `endpoint` parameter.
rg --type c --type h --word-regexp --fixed-strings --include '*.c' --include '*.h' 'oc_endpoint_log'

Length of output: 542


Script:

#!/bin/bash
# Description: Verify the implementation of `oc_endpoint_log`.

# Test: Search for the implementation of `oc_endpoint_log`. Expect: Correct handling of the `endpoint` parameter.
rg --type c --type h --word-regexp --fixed-strings -g '*.c' -g '*.h' 'void oc_endpoint_log(' -A 10

Length of output: 100


Script:

#!/bin/bash
# Description: Verify the implementation or definition of `oc_endpoint_log`.

# Test: Search for the implementation or definition of `oc_endpoint_log` in the codebase.
rg --type c --type h --word-regexp 'oc_endpoint_log' -A 10

Length of output: 3566

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7d64e76 and 89cdd06.

Files selected for processing (1)
  • port/linux/tcpsession.c (13 hunks)
Files skipped from review as they are similar to previous changes (1)
  • port/linux/tcpsession.c

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 89cdd06 and bee281a.

Files selected for processing (26)
  • api/cloud/oc_cloud.c (2 hunks)
  • api/cloud/oc_cloud_deregister.c (1 hunks)
  • api/cloud/oc_cloud_internal.h (1 hunks)
  • api/cloud/oc_cloud_manager.c (3 hunks)
  • api/oc_endpoint.c (2 hunks)
  • api/oc_endpoint_internal.h (2 hunks)
  • api/oc_event_callback.c (6 hunks)
  • api/oc_message.c (2 hunks)
  • api/oc_network_events.c (1 hunks)
  • api/oc_tcp.c (1 hunks)
  • api/oc_udp.c (1 hunks)
  • messaging/coap/log_internal.h (1 hunks)
  • port/android/ipadapter.c (4 hunks)
  • port/android/tcpadapter.c (3 hunks)
  • port/esp32/adapter/src/ipadapter.c (6 hunks)
  • port/esp32/adapter/src/tcpadapter.c (3 hunks)
  • port/linux/ip.c (1 hunks)
  • port/linux/ipadapter.c (10 hunks)
  • port/linux/tcpsession.c (17 hunks)
  • port/oc_log_internal.h (4 hunks)
  • port/openthread/ipadapter.c (3 hunks)
  • port/windows/ipadapter.c (5 hunks)
  • port/windows/tcpadapter.c (4 hunks)
  • port/zephyr/src/ipadapter.c (2 hunks)
  • security/oc_tls.c (14 hunks)
  • util/oc_etimer.c (5 hunks)
Files skipped from review due to trivial changes (7)
  • api/cloud/oc_cloud_deregister.c
  • api/oc_event_callback.c
  • api/oc_message.c
  • api/oc_tcp.c
  • api/oc_udp.c
  • port/linux/ip.c
  • port/linux/ipadapter.c
Files skipped from review as they are similar to previous changes (7)
  • api/cloud/oc_cloud.c
  • api/cloud/oc_cloud_manager.c
  • api/oc_endpoint.c
  • api/oc_endpoint_internal.h
  • api/oc_network_events.c
  • port/linux/tcpsession.c
  • security/oc_tls.c
Additional comments not posted (46)
messaging/coap/log_internal.h (2)

92-93: LGTM! Improved logging granularity.

The COAP_LOGipaddr macro now includes the log level and component, which enhances the logging granularity.


95-96: LGTM! Improved logging granularity.

The COAP_LOGipaddr_local macro now includes the log level and component, which enhances the logging granularity.

api/cloud/oc_cloud_internal.h (1)

130-133: LGTM! Ensure proper implementation.

The new function oc_cloud_endpoint_log is correctly declared for logging cloud endpoint addresses and session IDs.

However, ensure that the function is properly implemented and integrated with the existing code.

port/openthread/ipadapter.c (3)

63-63: LGTM! Improved logging granularity.

The OC_LOGipaddr call in udp_receive_cbk now includes the log level, which enhances the logging granularity.


114-114: LGTM! Improved logging granularity.

The OC_LOGipaddr call in oc_connectivity_get_endpoints now includes the log level, which enhances the logging granularity.


147-147: LGTM! Improved logging granularity.

The OC_LOGipaddr call in oc_send_buffer now includes the log level, which enhances the logging granularity.

util/oc_etimer.c (3)

53-53: LGTM! Logging level change from OC_DBG to OC_TRACE approved.

The shift from OC_DBG to OC_TRACE suggests a change in logging strategy, possibly to reduce verbosity or change the logging level.

Also applies to: 69-69


106-106: LGTM! Logging level change from OC_DBG to OC_TRACE approved.

The shift from OC_DBG to OC_TRACE suggests a change in logging strategy, possibly to reduce verbosity or change the logging level.

Also applies to: 116-116


181-181: LGTM! Logging level change from OC_DBG to OC_TRACE approved.

The shift from OC_DBG to OC_TRACE suggests a change in logging strategy, possibly to reduce verbosity or change the logging level.

port/zephyr/src/ipadapter.c (2)

133-133: LGTM! Added log level parameter to OC_LOGipaddr approved.

The addition of a log level parameter enhances logging granularity.


155-155: LGTM! Added log level parameter to OC_LOGipaddr approved.

The addition of a log level parameter enhances logging granularity.

port/oc_log_internal.h (6)

86-86: LGTM! Added log level parameter to OC_LOGipaddr approved.

The addition of a log level parameter enhances logging granularity.


230-231: LGTM! Added log level parameter to OC_LOGipaddr_WITH_COMPONENT approved.

The addition of a log level parameter enhances logging granularity.


234-235: LGTM! Added log level parameter to OC_LOGipaddr_local approved.

The addition of a log level parameter enhances logging granularity.


238-239: LGTM! Added log level parameter to OC_LOGipaddr_local_WITH_COMPONENT approved.

The addition of a log level parameter enhances logging granularity.


199-202: LGTM! Added log level parameter to OC_LOG_ENDPOINT_ADDR approved.

The addition of a log level parameter enhances logging granularity.

Also applies to: 214-215, 225-226


256-256: LGTM! Conditional inclusion of OC_LOGbytes_WITH_COMPONENT approved.

The change ensures that the logging functionality is only included when OC_TRACE_IS_ENABLED is true, optimizing performance.

port/esp32/adapter/src/tcpadapter.c (4)

284-286: Improvement: Added logging level for better granularity.

The addition of the logging level parameter to OC_LOGipaddr enhances the granularity and control of logging.


289-291: Improvement: Added logging level for better granularity.

The addition of the logging level parameter to OC_LOGipaddr enhances the granularity and control of logging.


415-416: Improvement: Replaced OC_DBG with OC_TRACE for better logging granularity.

The replacement of OC_DBG with OC_TRACE provides more detailed logging information, which can aid in debugging.


573-574: Improvement: Replaced OC_DBG with OC_TRACE for better logging granularity.

The replacement of OC_DBG with OC_TRACE provides more detailed logging information, which can aid in debugging.

port/android/tcpadapter.c (4)

313-315: Improvement: Added logging level for better granularity.

The addition of the logging level parameter to OC_LOGipaddr enhances the granularity and control of logging.


318-320: Improvement: Added logging level for better granularity.

The addition of the logging level parameter to OC_LOGipaddr enhances the granularity and control of logging.


442-443: Improvement: Replaced OC_DBG with OC_TRACE for better logging granularity.

The replacement of OC_DBG with OC_TRACE provides more detailed logging information, which can aid in debugging.


595-596: Improvement: Replaced OC_DBG with OC_TRACE for better logging granularity.

The replacement of OC_DBG with OC_TRACE provides more detailed logging information, which can aid in debugging.

port/windows/tcpadapter.c (5)

323-325: Improvement: Added logging level for better granularity.

The addition of the logging level parameter to OC_LOGipaddr enhances the granularity and control of logging.


328-330: Improvement: Added logging level for better granularity.

The addition of the logging level parameter to OC_LOGipaddr enhances the granularity and control of logging.


454-455: Improvement: Replaced OC_DBG with OC_TRACE for better logging granularity.

The replacement of OC_DBG with OC_TRACE provides more detailed logging information, which can aid in debugging.


488-489: Improvement: Replaced OC_DBG with OC_TRACE for better logging granularity.

The replacement of OC_DBG with OC_TRACE provides more detailed logging information, which can aid in debugging.


556-558: Improvement: Enhanced logging granularity.

Replaced OC_DBG with OC_TRACE for logging received messages and added a logging level to OC_LOGipaddr.

port/windows/ipadapter.c (5)

425-425: LGTM!

The addition of the log level to the OC_LOGipaddr function call is consistent with the new logging strategy.


443-443: LGTM!

The addition of the log level to the OC_LOGipaddr function call is consistent with the new logging strategy.


913-914: LGTM!

The addition of the log level to the OC_LOGipaddr function call is consistent with the new logging strategy.


1149-1149: LGTM!

The addition of the log level to the OC_LOGipaddr function call is consistent with the new logging strategy.


1161-1162: LGTM!

The addition of the log level to the OC_LOGipaddr function call is consistent with the new logging strategy.

port/android/ipadapter.c (4)

1009-1011: LGTM! Enhanced logging for incoming messages.

The shift from OC_DBG to OC_TRACE improves the granularity of logging for incoming messages, aiding in detailed tracking and debugging.


1054-1058: LGTM! Enhanced logging for event processing.

The shift from OC_DBG to OC_TRACE improves the granularity of logging for event processing, aiding in detailed monitoring and debugging.


1228-1228: LGTM! Enhanced logging for message sending.

The shift from OC_DBG to OC_TRACE improves the granularity of logging for message sending, aiding in detailed tracking and debugging.


1240-1242: LGTM! Enhanced logging for outgoing messages.

The shift from OC_DBG to OC_TRACE improves the granularity of logging for outgoing messages, aiding in detailed tracking and debugging.

port/esp32/adapter/src/ipadapter.c (7)

1147-1151: Verify the new log level parameter in OC_LOGipaddr and trace-level logging statements.

Ensure the log level parameter OC_LOG_LEVEL_TRACE is appropriate for this context and the trace-level logging statements provide useful information.

Verification successful
")` and `OC_TRACE("%s", "")` are also present, providing additional trace-level information.

[actions] Conclude the verification process by confirming that the new log level parameter and trace-level logging statements are used appropriately.
-->

The new log level parameter in OC_LOGipaddr and trace-level logging statements are used appropriately.

  • The OC_LOGipaddr function is correctly used with the OC_LOG_LEVEL_TRACE parameter.
  • Trace-level logging statements provide useful information in the context.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of OC_LOGipaddr with the new log level parameter and trace-level logging statements.

# Test: Search for the function usage. Expect: Only occurrences with the new log level parameter and trace-level logging statements.
rg --type c -A 5 $'OC_LOGipaddr'

Length of output: 13453


928-931: Verify the new log level parameter in OC_LOGipaddr and trace-level logging statements.

Ensure the log level parameter OC_LOG_LEVEL_TRACE is appropriate for this context and the trace-level logging statements provide useful information.

Verification successful

Verify the new log level parameter in OC_LOGipaddr and trace-level logging statements.

The new log level parameter OC_LOG_LEVEL_TRACE is used consistently across different files, including port/esp32/adapter/src/ipadapter.c. The trace-level logging statements provide useful information about the endpoint and interface index.

  • port/esp32/adapter/src/ipadapter.c: Lines 928-931
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of OC_LOGipaddr with the new log level parameter and trace-level logging statements.

# Test: Search for the function usage. Expect: Only occurrences with the new log level parameter and trace-level logging statements.
rg --type c -A 5 $'OC_LOGipaddr'

Length of output: 13453


471-471: Verify the new log level parameter in OC_LOGipaddr.

Ensure the log level parameter OC_LOG_LEVEL_DEBUG is appropriate for this context.

Verification successful

The log level parameter OC_LOG_LEVEL_DEBUG in OC_LOGipaddr is appropriate and consistent with other usages in the codebase.

  • port/esp32/adapter/src/ipadapter.c: Verified usage with OC_LOG_LEVEL_DEBUG.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of OC_LOGipaddr with the new log level parameter.

# Test: Search for the function usage. Expect: Only occurrences with the new log level parameter.
rg --type c -A 5 $'OC_LOGipaddr'

Length of output: 13453


494-494: Verify the new log level parameter in OC_LOGipaddr.

Ensure the log level parameter OC_LOG_LEVEL_DEBUG is appropriate for this context.


1135-1135: Verify the trace-level logging statement.

Ensure the trace-level logging statement OC_TRACE("Sent %d bytes", bytes_sent); provides useful information.


979-979: Verify the trace-level logging statement.

Ensure the trace-level logging statement OC_TRACE("processing %d events", fd_count); provides useful information.

Verification successful

Trace-level logging statement verified.

The trace-level logging statement OC_TRACE("processing %d events", fd_count); provides useful information by logging the number of events being processed, aiding in debugging and understanding the event flow.

  • port/esp32/adapter/src/ipadapter.c line 979
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the trace-level logging statement.

# Test: Search for the function usage. Expect: Only occurrences with the trace-level logging statement.
rg --type c -A 5 $'OC_TRACE("processing %d events", fd_count);'

Length of output: 64


Script:

#!/bin/bash
# Description: Verify the presence of the OC_TRACE macro and its usages.

# Search for the definition of the OC_TRACE macro.
rg --type c -A 5 $'#define OC_TRACE'

# Search for all usages of the OC_TRACE macro.
rg --type c -A 5 $'OC_TRACE('

Length of output: 1082


Script:

#!/bin/bash
# Description: Verify the presence of the OC_TRACE macro and its usages.

# Search for the definition of the OC_TRACE macro.
rg --type c -A 5 $'#define OC_TRACE'

# Search for all usages of the OC_TRACE macro with corrected regex pattern.
rg --type c -A 5 $'OC_TRACE\\('

# Specifically check the port/esp32/adapter/src/ipadapter.c file for the usage of OC_TRACE.
rg --type c -A 5 $'OC_TRACE\\(' port/esp32/adapter/src/ipadapter.c

Length of output: 24262


975-975: Verify the trace-level logging statement.

Ensure the trace-level logging statement OC_TRACE("process_events: timeout"); provides useful information.

@Danielius1922 Danielius1922 force-pushed the adam/feature/session_id_logs branch from bee281a to f9ecba3 Compare July 30, 2024 20:20
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bee281a and f9ecba3.

Files selected for processing (2)
  • api/oc_endpoint.c (2 hunks)
  • security/oc_tls.c (14 hunks)
Files skipped from review as they are similar to previous changes (1)
  • security/oc_tls.c
Additional comments not posted (2)
api/oc_endpoint.c (2)

736-737: LGTM! The change to ignore the next pointer is appropriate.

Ignoring the next pointer while checking if the endpoint is empty is a logical improvement.


823-838: LGTM! The new logging function is well-implemented.

The oc_endpoint_log function is efficiently designed to log endpoint details, including the session ID, only when debugging is enabled.

Ensure that all calls to oc_endpoint_log are correctly integrated into the codebase.

Verification successful

LGTM! The new logging function is well-implemented and integrated.

The oc_endpoint_log function is efficiently designed to log endpoint details, including the session ID, only when debugging is enabled. The function is correctly integrated into the codebase.

Locations:

  • api/oc_session_events.c
  • api/oc_client_api.c
  • api/cloud/oc_cloud.c
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `oc_endpoint_log` are correctly integrated.

# Test: Search for the function usage. Expect: Correct integration of the new logging function.
rg --type c -A 5 $'oc_endpoint_log'

Length of output: 1952

@Danielius1922 Danielius1922 added the OCF Conformance Testing OCF Conformance Testing required label Aug 1, 2024
@jkralik jkralik marked this pull request as ready for review August 5, 2024 13:22
@jkralik jkralik assigned jkralik and unassigned jkralik Aug 5, 2024
@jkralik jkralik self-requested a review August 5, 2024 13:22
@Danielius1922 Danielius1922 force-pushed the adam/feature/session_id_logs branch from f9ecba3 to a2fd42e Compare August 6, 2024 06:35
@ocf-conformance-test-tool ocf-conformance-test-tool bot removed the OCF Conformance Testing OCF Conformance Testing required label Aug 6, 2024
@Danielius1922
Copy link
Member Author

@coderabbitai generate a description for this pull request about targeting to general developer audience, not familiar with the code in this repository.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f9ecba3 and a2fd42e.

Files selected for processing (28)
  • api/cloud/oc_cloud.c (2 hunks)
  • api/cloud/oc_cloud_deregister.c (1 hunks)
  • api/cloud/oc_cloud_internal.h (1 hunks)
  • api/cloud/oc_cloud_manager.c (3 hunks)
  • api/oc_client_api.c (4 hunks)
  • api/oc_endpoint.c (2 hunks)
  • api/oc_endpoint_internal.h (2 hunks)
  • api/oc_event_callback.c (6 hunks)
  • api/oc_message.c (2 hunks)
  • api/oc_network_events.c (1 hunks)
  • api/oc_session_events.c (4 hunks)
  • api/oc_tcp.c (1 hunks)
  • api/oc_udp.c (1 hunks)
  • messaging/coap/log_internal.h (1 hunks)
  • port/android/ipadapter.c (4 hunks)
  • port/android/tcpadapter.c (3 hunks)
  • port/esp32/adapter/src/ipadapter.c (6 hunks)
  • port/esp32/adapter/src/tcpadapter.c (3 hunks)
  • port/linux/ip.c (1 hunks)
  • port/linux/ipadapter.c (10 hunks)
  • port/linux/tcpsession.c (17 hunks)
  • port/oc_log_internal.h (4 hunks)
  • port/openthread/ipadapter.c (3 hunks)
  • port/windows/ipadapter.c (5 hunks)
  • port/windows/tcpadapter.c (4 hunks)
  • port/zephyr/src/ipadapter.c (2 hunks)
  • security/oc_tls.c (14 hunks)
  • util/oc_etimer.c (5 hunks)
Files skipped from review due to trivial changes (3)
  • api/cloud/oc_cloud_manager.c
  • api/oc_event_callback.c
  • port/linux/ipadapter.c
Files skipped from review as they are similar to previous changes (25)
  • api/cloud/oc_cloud.c
  • api/cloud/oc_cloud_deregister.c
  • api/cloud/oc_cloud_internal.h
  • api/oc_client_api.c
  • api/oc_endpoint.c
  • api/oc_endpoint_internal.h
  • api/oc_message.c
  • api/oc_network_events.c
  • api/oc_session_events.c
  • api/oc_tcp.c
  • api/oc_udp.c
  • messaging/coap/log_internal.h
  • port/android/ipadapter.c
  • port/android/tcpadapter.c
  • port/esp32/adapter/src/ipadapter.c
  • port/esp32/adapter/src/tcpadapter.c
  • port/linux/ip.c
  • port/linux/tcpsession.c
  • port/oc_log_internal.h
  • port/openthread/ipadapter.c
  • port/windows/ipadapter.c
  • port/windows/tcpadapter.c
  • port/zephyr/src/ipadapter.c
  • security/oc_tls.c
  • util/oc_etimer.c

Copy link

coderabbitai bot commented Aug 6, 2024

@Danielius1922: Here is a description for the pull request targeting a general developer audience:


Description

This pull request enhances the logging capabilities of the codebase by adding session IDs to various endpoint logs. The main objective is to improve traceability and debugging by providing more detailed information about network and session events.

Key Changes

  1. Improved Logging:

    • Added session IDs to log messages related to endpoint activities, including session start, session end, and network events.
    • Introduced new logging functions to streamline and standardize how endpoint information is logged.
  2. Refactoring and Simplification:

    • Simplified the logging process by removing redundant code and using new logging functions.
    • Improved the clarity and consistency of log messages across different parts of the codebase.
  3. Enhanced Debugging:

    • Provided better context in log messages by including session IDs, making it easier to trace and debug issues related to network and session management.

These changes collectively aim to make the codebase more maintainable and easier to debug by providing developers with more detailed and consistent log information.


Actions performed

Comments resolved.

- add session_id to endpoint logs
- change log level of some logs, mostly DEBUG to TRACE
@Danielius1922 Danielius1922 force-pushed the adam/feature/session_id_logs branch from a2fd42e to 3b8b2a0 Compare August 6, 2024 06:52
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a2fd42e and 3b8b2a0.

Files selected for processing (28)
  • api/cloud/oc_cloud.c (2 hunks)
  • api/cloud/oc_cloud_deregister.c (1 hunks)
  • api/cloud/oc_cloud_internal.h (1 hunks)
  • api/cloud/oc_cloud_manager.c (3 hunks)
  • api/oc_client_api.c (4 hunks)
  • api/oc_endpoint.c (2 hunks)
  • api/oc_endpoint_internal.h (2 hunks)
  • api/oc_event_callback.c (6 hunks)
  • api/oc_message.c (2 hunks)
  • api/oc_network_events.c (1 hunks)
  • api/oc_session_events.c (4 hunks)
  • api/oc_tcp.c (1 hunks)
  • api/oc_udp.c (1 hunks)
  • messaging/coap/log_internal.h (1 hunks)
  • port/android/ipadapter.c (4 hunks)
  • port/android/tcpadapter.c (3 hunks)
  • port/esp32/adapter/src/ipadapter.c (6 hunks)
  • port/esp32/adapter/src/tcpadapter.c (3 hunks)
  • port/linux/ip.c (1 hunks)
  • port/linux/ipadapter.c (10 hunks)
  • port/linux/tcpsession.c (17 hunks)
  • port/oc_log_internal.h (4 hunks)
  • port/openthread/ipadapter.c (3 hunks)
  • port/windows/ipadapter.c (5 hunks)
  • port/windows/tcpadapter.c (4 hunks)
  • port/zephyr/src/ipadapter.c (2 hunks)
  • security/oc_tls.c (14 hunks)
  • util/oc_etimer.c (5 hunks)
Files skipped from review due to trivial changes (4)
  • api/cloud/oc_cloud_deregister.c
  • api/oc_event_callback.c
  • port/android/tcpadapter.c
  • port/linux/ipadapter.c
Files skipped from review as they are similar to previous changes (23)
  • api/cloud/oc_cloud.c
  • api/cloud/oc_cloud_manager.c
  • api/oc_client_api.c
  • api/oc_endpoint.c
  • api/oc_endpoint_internal.h
  • api/oc_message.c
  • api/oc_network_events.c
  • api/oc_session_events.c
  • api/oc_tcp.c
  • api/oc_udp.c
  • messaging/coap/log_internal.h
  • port/android/ipadapter.c
  • port/esp32/adapter/src/ipadapter.c
  • port/esp32/adapter/src/tcpadapter.c
  • port/linux/ip.c
  • port/linux/tcpsession.c
  • port/oc_log_internal.h
  • port/openthread/ipadapter.c
  • port/windows/ipadapter.c
  • port/windows/tcpadapter.c
  • port/zephyr/src/ipadapter.c
  • security/oc_tls.c
  • util/oc_etimer.c
Additional comments not posted (1)
api/cloud/oc_cloud_internal.h (1)

130-133: LGTM! The new function declaration is appropriate.

The function oc_cloud_endpoint_log is well-named and its parameters are descriptive. The use of OC_NONNULL ensures safety by enforcing non-null parameters.

@Danielius1922 Danielius1922 added the OCF Conformance Testing OCF Conformance Testing required label Aug 6, 2024
Copy link

sonarqubecloud bot commented Aug 6, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
8.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@Danielius1922 Danielius1922 merged commit c4588fb into master Aug 6, 2024
131 of 132 checks passed
@Danielius1922 Danielius1922 deleted the adam/feature/session_id_logs branch August 6, 2024 13:06
@github-actions github-actions bot locked and limited conversation to collaborators Aug 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
OCF Conformance Testing OCF Conformance Testing required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants