feat(client): add usage reporting #2369

62w71st · 2025-02-07T20:23:56Z

No description provided.

…into usage-reporting-v2

…ne-apps into usage-reporting-v2

…into usage-reporting-v2

client/go/outline/config/config_tcpudp.go

client/src/www/app/outline_server_repository/vpn.cordova.ts

...c/cordova/android/OutlineAndroidLib/outline/src/main/aidl/org/outline/IVpnTunnelService.aidl

client/go/outline/client.go

client/go/outline/config/config_usage_reporter.go

jyyi1

Thanks for the contribution! As discussed in the meeting, please:

Refactor the newTunnel to accept a Client parameter
Parse the reporting config in NewClientWithBaseDialers, we can add new reporting field to type Client struct
Start the reporting here or here by reading the parsed reporting config field in Client. Maybe the latter one is better because we will add "Stop reporting" logic in the same file, but this means we need to pass additional reporting config parameter to Tunnel, this is a trade-off.
Stop the reporting here

Please also fix the go tests, the CI seems to be failing.

client/go/outline/config/config_usage_reporter.go

client/go/outline/config/config_tcpudp.go

client/go/outline/reporting/cookies.json

...nt/src/cordova/android/OutlineAndroidLib/outline/src/main/aidl/org/outline/TunnelConfig.aidl

...ordova/android/OutlineAndroidLib/outline/src/main/java/org/outline/vpn/VpnTunnelService.java

client/src/cordova/plugin/android/java/org/outline/OutlinePlugin.java

client/go/outline/reporting/reporting_client.go

…om the // Android plugin.

…into usage-reporting-v2

62w71st

PTAL

client/go/outline/config/config_tcpudp.go

client/src/cordova/plugin/android/java/org/outline/OutlinePlugin.java

client/src/www/app/outline_server_repository/vpn.cordova.ts

client/go/outline/config/config_tcpudp.go

...nt/src/cordova/android/OutlineAndroidLib/outline/src/main/aidl/org/outline/TunnelConfig.aidl

client/go/outline/client.go

client/go/outline/config/config_usage_reporter.go

client/go/outline/reporting/reporting_client.go

jyyi1

Thanks for the update! The code looks a lot better.

jyyi1 · 2025-05-16T20:05:11Z

client/go/outline/client.go

@@ -7,7 +7,7 @@
 //      http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
+// distributed under the License is distributed on an "AS IS BASIS,


Plz revert

Suggested change

// distributed under the License is distributed on an "AS IS BASIS,

// distributed under the License is distributed on an "AS IS" BASIS,

jyyi1 · 2025-05-16T20:08:13Z

client/go/outline/client.go

+	if err != nil {
+		return nil, &platerrors.PlatformError{
+			Code:    platerrors.InvalidConfig,
+			Message: "config is not valid YAML",


Make the error message different from the previous one so the user knows what's going wrong.

Suggested change

Message: "config is not valid YAML",

Message: "client config is not valid YAML",

jyyi1 · 2025-05-16T20:09:06Z

client/go/outline/client.go

+		if errors.Is(err, errors.ErrUnsupported) {
+			return nil, &platerrors.PlatformError{
+				Code:    platerrors.InvalidConfig,
+				Message: "unsupported config",


Same as above.

Suggested change

Message: "unsupported config",

Message: "unsupported client config",

jyyi1 · 2025-05-16T20:11:54Z

client/go/outline/client.go

@@ -101,5 +126,31 @@ func NewClientWithBaseDialers(transportConfig string, tcpDialer transport.Stream
 		}
 	}

-	return &Client{sd: transportPair.StreamDialer, pl: transportPair.PacketListener}, nil
+	usageReportYAML, err := config.ParseConfigYAML(sessionConfig)


Is this sessionConfig specific to sessionReportingConfig? Or can it be used for other types of sessionConfig in the future? If it's the latter, we should probably implement type assertion to ensure the type is 'reporting'.

I think the easiest is to move the parsing of the clientConfig to the config module, which already takes care of this. And it allows you to use first_supported

jyyi1 · 2025-05-16T20:16:47Z

client/go/outline/client.go

+			}
+		}
+	}
+	fmt.Println("usageReporter", usageReporter)


Is this for debugging purpose or logging purpose?

jyyi1 · 2025-05-16T21:15:18Z

client/go/outline/reporting/reporting_client.go

+						Name:  cookieParts[0],
+						Value: cookieParts[1],
+					})
+					fmt.Println("Cookie found:", cookie)


logging/debugging?

jyyi1 · 2025-05-16T21:18:08Z

client/go/outline/reporting/reporting_client.go

+func loadCookies(jar http.CookieJar, filename string) ([]*url.URL, error) {
+	// Read cookies from the file
+	file, err := os.Open(filename)
+	if os.IsNotExist(err) {


According to Go doc:

This function predates errors.Is. It only supports errors returned by the os package. New code should use errors.Is(err, fs.ErrNotExist).

Suggested change

if os.IsNotExist(err) {

if errors.Is(err, fs.ErrNotExist) {

jyyi1 · 2025-05-16T21:19:36Z

client/go/outline/reporting/reporting_client.go

+	}
+
+	// Save the cookies data to a file
+	file, err := os.Create(filename)


As we discussed for privacy, do we need to allow the user to delete this cookie file (though removing the key or any other methods)?

The cookie can be deleted when the key is deleted or when the Outline app is uninstalled. Do you want to add the option to be able to delete cookies manually?
@62w71st @fortuna

jyyi1 · 2025-05-16T21:24:44Z

client/go/outline/reporting/reporting_client.go

+			}
+		case <-ctx.Done():
+			// Stop reporting when the context is canceled
+			fmt.Println("Stopping reporting...")


jyyi1 · 2025-05-16T21:24:52Z

client/go/outline/reporting/reporting_client.go

+		case <-ticker.C:
+			err := Report(tcp, ur.Url)
+			if err != nil {
+				fmt.Printf("Report failed: %v\n", err)


Plz use slog for logging?

fortuna · 2025-05-19T15:20:38Z

...ordova/android/OutlineAndroidLib/outline/src/main/java/org/outline/vpn/VpnTunnelService.java

+
+    // Stop reporting on the Client instance
+    if (this.outlineClient != null) {
+        this.outlineClient.stopReporting();


Move to tearDownActiveTunnel. But I wonder if we should just implement that in stopRemoteDevice instead and revert this file. Why would we not align them?

client/go/outline/vpn_linux.go

fortuna · 2025-05-19T15:26:14Z

client/go/outline/client.go

@@ -41,6 +45,10 @@ func (c *Client) ListenPacket(ctx context.Context) (net.PacketConn, error) {
 	return c.pl.ListenPacket(ctx)
 }

+func (c *Client) GetUsageReporter() *config.UsageReporter {


Why is this needed? You already provide Start/StopReporting. Remove?

client/go/outline/client.go

fortuna · 2025-05-19T15:29:52Z

client/go/outline/client.go

 // NewClient creates a new Outline client from a configuration string.
-func NewClient(transportConfig string) *NewClientResult {
+func NewClient(transportConfig string, sessionConfig string) *NewClientResult {


This is not a session config. It's a report config. But we should merge them into a client config.

fortuna · 2025-05-19T15:30:00Z

client/go/outline/client.go

 	if err != nil {
 		return &NewClientResult{Error: platerrors.ToPlatformError(err)}
 	}
 	return &NewClientResult{Client: client}
 }

-func NewClientWithBaseDialers(transportConfig string, tcpDialer transport.StreamDialer, udpDialer transport.PacketDialer) (*Client, error) {
+func NewClientWithBaseDialers(transportConfig string, sessionConfig string, tcpDialer transport.StreamDialer, udpDialer transport.PacketDialer) (*Client, error) {


fortuna · 2025-05-19T15:33:16Z

client/go/outline/client.go

@@ -101,5 +126,31 @@ func NewClientWithBaseDialers(transportConfig string, tcpDialer transport.Stream
 		}
 	}

-	return &Client{sd: transportPair.StreamDialer, pl: transportPair.PacketListener}, nil
+	usageReportYAML, err := config.ParseConfigYAML(sessionConfig)


I think the easiest is to move the parsing of the clientConfig to the config module, which already takes care of this. And it allows you to use first_supported

client/src/www/app/outline_server_repository/vpn.cordova.ts

fortuna · 2025-05-19T15:59:41Z

client/src/www/app/outline_server_repository/config.ts

  transport: string;
+  session_report: string;


Suggested change

transport: string;

session_report: string;

client: string;

fortuna · 2025-05-19T16:31:08Z

client/go/outline/reporting/reporting_client.go

+	defer conn.Close()
+
+	// Create an HTTP POST request with cookies and a sample body
+	requestBody := "param1=value1&param2=value2"


Please use the http.Client instead of rolling out your own.

fortuna · 2025-05-19T16:34:05Z

client/go/outline/reporting/reporting_client.go

+}
+
+// StartReporting calls the Report function at every internal.
+func StartReporting(ctx context.Context, tcp transport.StreamDialer, ur *config.UsageReporter) {


Cookies must be siloed across access keys. You must take into account the service id somehow.

Current, cookies for all keys belonging to a provider are sent in the request. The provider should be able to dedupe them. Can you explain the requirement that motivates separating these cookies per key?

Each access key must have their own separate cookie jar to prevent possible cross-service leaks. It's a security requirement. Imagine two separate services talking to the same reporting service, for example.

Also, you need to delete the cookies when you delete the key, and this makes it easier, because you don't know the domains it needs to talk to.

…ne-apps into usage-reporting-v2

fortuna · 2025-06-03T21:25:50Z

Please pull the latest master

62w71st and others added 21 commits July 16, 2024 23:44

Pass in serverName to VpnTunnelService

58484de

Pass in serverName to VpnTunnelService

304748c

merge

ec80311

Merge branch 'master' of https://github.com/62w71st/outline-apps

bd4c0f9

merge

b5ce091

add servername to cordova and electron

2716f8b

change the origin of the outline plugin code

85e688f

conditionally pass serverName for Cordova only on Android.

321868f

lint

e44ced5

merge with upstream

3ad060e

resolve conflicts

0ea80b0

conflict resolution part 2.

337695c

fix compilation errors

b4e2c51

Merge branch 'master' of https://github.com/62w71st/outline-apps

43a9b80

Add serverName to logs and remove getServerName

ecaedf2

remove ongoing

eb0316c

Merge branch 'master' into master

fb2080e

Merge branch 'master' of https://github.com/Jigsaw-Code/outline-apps

5b9acb1

Add reporting client.

225b9da

usage-reporting parser.

8ba5d02

Merge remote-tracking branch 'upstream/master' into usage-reporting-v2

ca66235

62w71st requested a review from a team as a code owner February 7, 2025 20:23

62w71st added 6 commits February 17, 2025 14:39

Merge branch 'Jigsaw-Code:master' into usage-reporting-v2

fc21f96

Merge branch 'master' of https://github.com/Jigsaw-Code/outline-apps …

21dd88f

…into usage-reporting-v2

Merge branch 'usage-reporting-v2' of https://github.com/62w71st/outli…

2e46848

…ne-apps into usage-reporting-v2

GET -> POST

56d5cfe

Merge branch 'master' of https://github.com/Jigsaw-Code/outline-apps …

3aca920

…into usage-reporting-v2

Add OnConnected() callback.

3e1da1f

62w71st requested a review from a team as a code owner February 22, 2025 21:35

fix build

6718e50

62w71st added 3 commits April 18, 2025 19:18

Fix build.

f8aaa80

Fix Getter.

42a20de

gobind

341455e

fortuna requested changes Apr 24, 2025

View reviewed changes

jyyi1 reviewed Apr 24, 2025

View reviewed changes

jyyi1 changed the title ~~(feat) Usage Reporting~~ feat(client): add usage reporting Apr 24, 2025

62w71st added 10 commits April 26, 2025 20:06

Merge branch 'Jigsaw-Code:master' into usage-reporting-v2

5c1185b

Addressed a few comments and removed usage reporting configuration fr…

a1c3bed

…om the // Android plugin.

Removed usage reporting config from TunnelConfig.aidl

ed0d9d4

Merge branch 'master' of https://github.com/Jigsaw-Code/outline-apps …

78449ea

…into usage-reporting-v2

remove cookies.json

e6f0175

merge upstream

0bf84ae

Fix tests.

edc200a

Pass in session_report to the cordova plugin.

5d9a16c

Fix module_test.go tests.

b7f529c

stop reporting when tunnel is stopped.

c7dbf80

62w71st commented May 15, 2025

View reviewed changes

Merge branch 'master' into usage-reporting-v2

b14c48a

emohandesi mentioned this pull request May 16, 2025

refactor(client): use client when creating a new tunnel #2498

Merged

jyyi1 reviewed May 16, 2025

View reviewed changes

fortuna requested changes May 19, 2025

View reviewed changes

fortuna mentioned this pull request May 23, 2025

Enable user to report the connection failure #2036

Open

1 task

62w71st added 4 commits May 26, 2025 13:32

Merge branch 'Jigsaw-Code:master' into usage-reporting-v2

896c7b9

Apply comments.

b33ec07

Merge branch 'usage-reporting-v2' of https://github.com/62w71st/outli…

557972b

…ne-apps into usage-reporting-v2

Apply comments.

ecdcef1

Merge remote-tracking branch 'upstream/master' into usage-reporting-v2

4e75174

	// distributed under the License is distributed on an "AS IS BASIS,
	// distributed under the License is distributed on an "AS IS" BASIS,

	Message: "config is not valid YAML",
	Message: "client config is not valid YAML",

	Message: "unsupported config",
	Message: "unsupported client config",

feat(client): add usage reporting #2369

Are you sure you want to change the base?

feat(client): add usage reporting #2369

Uh oh!

Conversation

62w71st commented Feb 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyyi1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

62w71st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyyi1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jyyi1 May 16, 2025 • edited by fortuna Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jyyi1 May 16, 2025 • edited by fortuna Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jyyi1 left a comment •

edited

Loading

jyyi1 May 16, 2025 •

edited by fortuna

Loading

jyyi1 May 16, 2025 •

edited by fortuna

Loading