Skip to content

feat: add timestamp for DAP trace and formate output using JSON.stringify(message, null, 4)#10484

Merged
vince-fugnitto merged 5 commits intoeclipse-theia:masterfrom
DanboDuan:main
Dec 2, 2021
Merged

feat: add timestamp for DAP trace and formate output using JSON.stringify(message, null, 4)#10484
vince-fugnitto merged 5 commits intoeclipse-theia:masterfrom
DanboDuan:main

Conversation

@DanboDuan
Copy link
Contributor

@DanboDuan DanboDuan commented Nov 27, 2021

ADD timestamp for DAP trace output

DAP trace output does not contain timestamp. it looks like this

fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305159},"event":"thread","seq":0,"type":"event"}
fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305187},"event":"thread","seq":0,"type":"event"}
fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305194},"event":"thread","seq":0,"type":"event"}
fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305286},"event":"thread","seq":0,"type":"event"}
fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305343},"event":"thread","seq":0,"type":"event"}
fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305191},"event":"thread","seq":0,"type":"event"}
fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305281},"event":"thread","seq":0,"type":"event"}
fabf8f58 theia -> adapter: {"seq":18,"type":"request","command":"threads","arguments":{}}

It's not easy to get the performance with those trace output. It's better to output a timestamp so you will be easy to get a context.

How to test

check the dap trace output is now with timestamp like this

fabf8f58 theia <- adapter: {"body":{"reason":"exited","threadId":2305159},"event":"thread","seq":0,"type":"event"}

after the changes submit, check the dap trace output is now with timestamp like this

fabf8f58 2021-11-27T02:40:06.428Z theia <- adapter: {"body":{"reason":"exited","threadId":2305159},"event":"thread","seq":0,"type":"event"}

Review checklist

Reminder for reviewers

  • As a reviewer, I agree to behave in accordance with the review guidelines

Signed-off-by: bob <bob170731@gmail.com>
@msujew msujew added dap debug adapter protocol logging issues related to logging labels Nov 29, 2021
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

Hi @DanboDuan, thank you for your first contribution.

the improvement to logging looks good. I have a suggestion to simplify the change.

Signed-off-by: bob <bob170731@gmail.com>
Signed-off-by: bob <bob170731@gmail.com>
@DanboDuan DanboDuan changed the title feat: add timestamp for DAP trace feat: add timestamp for DAP trace and formate output using JSON.stringify(message, null, 4) Nov 30, 2021
Signed-off-by: bob <bob170731@gmail.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that the changes work well, and the timestamp is now added to the traces of the debug adapter. Example:

ab3a895f 12/1/2021, 08:50:30.582 theia -> adapter: {
    "seq": 34,
    "type": "request",
    "command": "scopes",
    "arguments": {
        "frameId": 34
    }
}

Signed-off-by: bob <bob170731@gmail.com>
@vince-fugnitto
Copy link
Member

@msujew any objections? :)

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

@vince-fugnitto None, looks good to me 👍

@vince-fugnitto vince-fugnitto merged commit f97f787 into eclipse-theia:master Dec 2, 2021
@vince-fugnitto vince-fugnitto added this to the 1.21.0 milestone Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dap debug adapter protocol logging issues related to logging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants