NotificationsYou must be signed in to change notification settings
Fork914
Star10.1k

fix: track JetBrains connections#10968

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

code-asher merged 12 commits intomainfromjetbrains/track_connections

Dec 7, 2023

Merged

fix: track JetBrains connections#10968

code-asher merged 12 commits intomainfromjetbrains/track_connections

Dec 7, 2023

Conversation

Copy link

Member

code-asher commentedDec 1, 2023•
edited
Loading

Fixes#9673

Possibly we will need to find a better string to match on, but this one seems OK to me.

Unit test passes but have not tested an actual deploy yet.

Emyrk added3 commits

November 30, 2023 16:08

feat: implement jetbrains agentssh tracking

c9a226f

Based on tcp forwarding instead of ssh connections

Add unit test to confirm tracking

2447970

implement unit test to verify jetbrains functionality

76d3a24

github-actionsbot assignedcode-asher

Dec 1, 2023

code-asher changed the title~~Track JetBrains connections~~fix: track JetBrains connections

Dec 1, 2023

code-asher force-pushed thejetbrains/track_connections branch from7948ab5 toe7f9516Compare

December 1, 2023 00:24

Copy link

Member

matifali commentedDec 1, 2023

@code-asher Try./scripts/deploy-pr.sh

Copy link

MemberAuthor

code-asher commentedDec 1, 2023

Verified it works! 🎉

code-asher marked this pull request as ready for review

December 1, 2023 19:50

code-asher requested a review fromEmyrk

December 1, 2023 19:53

code-asher force-pushed thejetbrains/track_connections branch 2 times, most recently fromfe83fbd to9dede7fCompare

December 1, 2023 20:18

code-asher removed the request for review fromEmyrk

December 1, 2023 20:19

Copy link

MemberAuthor

code-asher commentedDec 1, 2023

Ah wait moving back to draft, there seems to be an issue...

code-asher marked this pull request as draft

December 1, 2023 20:20

Emyrk reviewed

Dec 1, 2023

View reviewed changes

agent/agent_test.go OutdatedShow resolvedHide resolved

agent/agentssh/jetbrainstrack.goShow resolvedHide resolved

agent/agentssh/jetbrainstrack.go OutdatedShow resolvedHide resolved

code-asher force-pushed thejetbrains/track_connections branch from9dede7f to9840a69Compare

December 1, 2023 20:45

Copy link

MemberAuthor

code-asher commentedDec 1, 2023

The two issues I ran into, just FYI:

I had the process name check reversed, needed a! 🤦
Netstat only gives the base name, not the full command line, so I need to read/proc/$pid/cmdline to get that.

I think it is all sorted now, doing some more testing to be sure.

Copy link

MemberAuthor

code-asher commentedDec 1, 2023

OK, verified it all works as expected in a deployment. Test is failing in CI though. Passes for me locally...

Copy link

Member

Emyrk commentedDec 1, 2023

Test is failing in CI though. Passes for me locally...

Ooof, this test is quite strange too. That sounds annoying to debug...

Uset.Log() to log statements, and keep them there.

code-asherand others added6 commits

December 1, 2023 12:05

Implement port process inspection

adf2fb3

Add JetBrains tracking to bottom bar

ad034f2

Elaborate on process name check comment

34b7c5e

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>

Comment that localForwardChannelData is copied

dce56fd

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>

Comment ChannelAccepterWatcher

7139448

Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>

Rename channel watcher to be specific to Jetbrains

6e8f235

code-asher force-pushed thejetbrains/track_connections branch from64ea05d to6e8f235Compare

December 1, 2023 21:06

Log unmarshal failure

4d65478

Copy link

Member

matifali commentedDec 1, 2023

Will this also work for Fleet?

code-asher force-pushed thejetbrains/track_connections branch frome79b01c toec59e7eCompare

December 1, 2023 21:27

Copy link

MemberAuthor

code-asher commentedDec 1, 2023

Will this also work for Fleet?

I am not sure, we will need to test.

code-asher force-pushed thejetbrains/track_connections branch fromec59e7e to90e6e88Compare

December 1, 2023 21:43

Add constant for JetBrains magic string

254a5b6

code-asher force-pushed thejetbrains/track_connections branch 2 times, most recently from9cf5082 to7fd04cbCompare

December 1, 2023 23:19

Fix JetBrains tracking test

a75ed6c

The test name only shows up in the process name if you are running thattest directly so we have to spawn a separate process instead.

code-asher force-pushed thejetbrains/track_connections branch from7fd04cb toa75ed6cCompare

December 1, 2023 23:26

Copy link

MemberAuthor

code-asher commentedDec 1, 2023

OK, sorted the test. The test name only shows up in the process name if you run that specific test directly, so when we are running the entire suite the name does not show up.

What I did was spawn an external process instead. Not sure if I did it in an idiomatic way or if there even is an idiomatic way, with Node I would justfork to create child processes using the same Node binary but I am not seeing an equivalent in Go, at least not without a new package dependency. So I have this jankyexec.Command("go", "run", ...) type of deal.

We could also try spawning something likenc instead of a Go program. but I am not sure how commonlync is installed.