Skip to content

Improved gRPC debug logger #1730

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 11, 2022
Merged

Improved gRPC debug logger #1730

merged 1 commit into from
May 11, 2022

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented May 11, 2022

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)

What kind of change does this PR introduce?
Since many gRPC calls may happen together, to better understand what's happening, now:

  • each call is identified by a sequence number (even if they are interleaved)
  • when a non-streaming calls ends a "CALL END" log is printed

for example:

14 CALLED: /cc.arduino.cli.commands.v1.ArduinoCoreService/Init STREAM_RESP
14 |  REQ:  {
14 |    "instance": {
14 |      "id": 1
14 |    }
14 |  }
14 |  RESP: {
14 |    "Message": {
14 |      "Error": {
14 |        "code": 9,
14 |        "message": "Loading index file: loading json index file /tmp/arduino-rpc-client4151737853/package_index.json: open /tmp/arduino-rpc-client4151737853/package_index.json: no such file or directory"
14 |      }
14 |    }
14 |  }
14 |  RESP: {
14 |    "Message": {
14 |      "Error": {
14 |        "code": 9,
14 |        "message": "Loading index file: loading json index file /tmp/arduino-rpc-client4151737853: read /tmp/arduino-rpc-client4151737853: is a directory"
14 |      }
14 |    }
14 |  }
14 |  RESP: {
14 |    "Message": {
14 |      "Error": {
14 |        "code": 9,
14 |        "message": "Error loading hardware platform: discovery builtin:serial-discovery not found"
14 |      }
14 |    }
14 |  }
14 |  RESP: {
14 |    "Message": {
14 |      "Error": {
14 |        "code": 9,
14 |        "message": "Error loading hardware platform: discovery builtin:mdns-discovery not found"
14 |      }
14 |    }
14 |  }
14 |  RESP: {
14 |    "Message": {
14 |      "Error": {
14 |        "code": 9,
14 |        "message": "Loading index file: reading library_index.json: open /tmp/arduino-rpc-client4151737853/library_index.json: no such file or directory"
14 |      }
14 |    }
14 |  }
14 STREAM CLOSED

15 CALLED: /cc.arduino.cli.settings.v1.SettingsService/SetValue
15 |  REQ:  {
15 |    "key": "network.proxy",
15 |    "json_data": "\"http://localhost:3128\""
15 |  }
15 |  RESP: {}
15 CALL END

16 CALLED: /cc.arduino.cli.commands.v1.ArduinoCoreService/UpdateIndex STREAM_RESP
16 |  REQ:  {
16 |    "instance": {
16 |      "id": 1
16 |    }
16 |  }
16 |  ERROR:  rpc error: code = Internal desc = Error downloading index 'https://downloads.arduino.cc/packages/package_index.json': Get "https://downloads.arduino.cc/packages/package_index.json": proxyconnect tcp: dial tcp 127.0.0.1:3128: connect: connection refused
16 STREAM CLOSED

What is the current behavior?
If many streaming gRPC calls happens together the output is mixed.

What is the new behavior?
Each call has his unique sequence number, we now identify each message to which call belongs.

Does this PR introduce a breaking change, and is titled accordingly?
no breaking changes.

@cmaglie
Improved gRPC debug logger
dfa5faf 
Since many gRPC calls may happen together, to better understand what's
happening now:

- each call is identified by a sequence number (even if they are
  interleaved)
- when a non-streaming calls ends a "CALL END" log is printed
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself topic: gRPC Related to the gRPC interface labels May 11, 2022
@umbynos umbynos self-requested a review May 11, 2022 10:44
umbynos
umbynos approved these changes May 11, 2022
View reviewed changes
Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

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

LGTM

@cmaglie cmaglie merged commit b6d36c6 into arduino:master May 11, 2022
@cmaglie cmaglie deleted the improved_grpc_debug branch May 11, 2022 10:48
@cmaglie cmaglie self-assigned this May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@umbynos umbynos umbynos approved these changes

Assignees

@cmaglie cmaglie

Labels
topic: code Related to content of the project itself topic: gRPC Related to the gRPC interface type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cmaglie @umbynos @per1234