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

Adding Correlation id to the logs #1906

Merged
merged 5 commits into from
Jan 23, 2023

Conversation

asoto-iov
Copy link
Contributor

@asoto-iov asoto-iov commented Oct 14, 2022

Adding correlation id's to the logs in order to have a better tracking.
To see the logs it would be helpful to set the following log levels:
web3 -> DEBUG
net -> TRACE
wire -> TRACE
messagehandler -> TRACE

Description

The idea is to create unique identifiers for each action and use it to relate the different logged data along the code.

This is a draft to test possible solutions in different areas/modules.

Motivation and Context

Now is hard and track different actions by checking the logs as similar actions are being executed in parallel by different threads.

How Has This Been Tested?

It doesn't have side effects in other areas.
[JSON-RPC]
To check it you can send any JSON-RPC request and check the trace.id value in the logs.
The trace id is being generated randomly but you can add a custom one by adding a custom header X-TRACE-ID in the request.

curl --location --request POST 'http://localhost:4444' \ --header 'Content-Type: application/json' \ --header 'X-TRACE-ID: custom_header_value' \ --data-raw '{ "jsonrpc":"2.0", "method":"web3_clientVersion", "params":[], "id":1 }'

Types of changes

It is a new feature

Checklist:

  • [x ] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

@asoto-iov asoto-iov force-pushed the feature/adding_correlation_id_to_logs branch 2 times, most recently from 36ffca8 to 2cab4ab Compare October 28, 2022 16:35
@Vovchyk
Copy link
Contributor

Vovchyk commented Oct 31, 2022

@asoto-iov , tests are failing. Please check

@asoto-iov asoto-iov marked this pull request as ready for review October 31, 2022 09:12
@Vovchyk
Copy link
Contributor

Vovchyk commented Oct 31, 2022

pipeline:run

@@ -0,0 +1,18 @@
package co.rsk.util;
Copy link
Contributor

Choose a reason for hiding this comment

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

cc header is missing

class TraceUtilsTest{

@Test
void testToId() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we should also cover other cases, eg. when the test value is null, its length is less than 15 etc

@sonarcloud
Copy link

sonarcloud bot commented Oct 31, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 1 Code Smell

92.1% 92.1% Coverage
0.0% 0.0% Duplication

Vovchyk
Vovchyk previously approved these changes Oct 31, 2022
@Vovchyk
Copy link
Contributor

Vovchyk commented Oct 31, 2022

pipeline:run

3 similar comments
@asoto-iov
Copy link
Contributor Author

pipeline:run

@Vovchyk
Copy link
Contributor

Vovchyk commented Oct 31, 2022

pipeline:run

@Vovchyk
Copy link
Contributor

Vovchyk commented Oct 31, 2022

pipeline:run

RskMessage rskmessage = (RskMessage) msg;
Message message = rskmessage.getMessage();

switch (message.getMessageType()) {

Check warning

Code scanning / CodeQL

Missing enum case in switch

Switch statement does not have a case for [NEW_BLOCK_HASH_MESSAGE](1). Switch statement does not have a case for [SKELETON_REQUEST_MESSAGE](2). Switch statement does not have a case for [BODY_RESPONSE_MESSAGE](3). Switch statement does not have a case for [BODY_REQUEST_MESSAGE](4). Switch statement does not have a case for [SKELETON_RESPONSE_MESSAGE](5). Switch statement does not have a case for [BLOCK_RESPONSE_MESSAGE](6). Switch statement does not have a case for [BLOCK_REQUEST_MESSAGE](7). Switch statement does not have a case for [BLOCK_HEADERS_RESPONSE_MESSAGE](8). Switch statement does not have a case for [BLOCK_HEADERS_REQUEST_MESSAGE](9). Switch statement does not have a case for [BLOCK_HASH_RESPONSE_MESSAGE](10). Switch statement does not have a case for [BLOCK_HASH_REQUEST_MESSAGE](11). Switch statement does not have a case for [TRANSACTIONS](12). Switch statement does not have a case for [NEW_BLOCK_HASHES](13).

import static org.junit.jupiter.api.Assertions.*;

class TraceUtilsTest {

Check notice

Code scanning / CodeQL

Unused classes and interfaces

Unused class: TraceUtilsTest is not referenced within this codebase. If not used as an external API it should be removed.
@sonarcloud
Copy link

sonarcloud bot commented Jan 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 1 Code Smell

92.1% 92.1% Coverage
0.0% 0.0% Duplication

@Vovchyk
Copy link
Contributor

Vovchyk commented Jan 23, 2023

pipeline:run

@Vovchyk Vovchyk merged commit ae70717 into master Jan 23, 2023
@Vovchyk Vovchyk deleted the feature/adding_correlation_id_to_logs branch January 23, 2023 15:52
@aeidelman aeidelman added this to the Hop 4.3.0 milestone Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants