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

Send connectivity error report to specified collector destination #90

Closed
wants to merge 33 commits into from

Conversation

amircybersec
Copy link
Contributor

I updated the Outline connectivity code to accept a new optional `-collector' flag for a URL to send the error report JSON object with a POST call. I tested this with report collector I setup using Apps Script. More info on collector setup can be found here.

Copy link
Contributor

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

Nice job, I'm loving the progress!

x/examples/outline-connectivity/main.go Outdated Show resolved Hide resolved
x/examples/outline-connectivity/main.go Outdated Show resolved Hide resolved
x/examples/outline-connectivity/main.go Outdated Show resolved Hide resolved
x/examples/outline-connectivity/main.go Outdated Show resolved Hide resolved
amircybersec and others added 4 commits October 12, 2023 17:20
change collector flag to report-to

Co-authored-by: Vinicius Fortuna <fortuna@users.noreply.github.com>
change NewBuffer to NewReader

Co-authored-by: Vinicius Fortuna <fortuna@users.noreply.github.com>
Copy link
Contributor

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

It looks like you need to resolve conflicts with the main branch

@@ -83,12 +85,44 @@ func unwrapAll(err error) error {
}
}

func sendReport(record jsonRecord, collectorFlag *string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is clearer:

Suggested change
func sendReport(record jsonRecord, collectorFlag *string) error {
func sendReport(record jsonRecord, collectorURL string) error {

func main() {
verboseFlag := flag.Bool("v", false, "Enable debug output")
transportFlag := flag.String("transport", "", "Transport config")
domainFlag := flag.String("domain", "example.com.", "Domain name to resolve in the test")
resolverFlag := flag.String("resolver", "8.8.8.8,2001:4860:4860::8888", "Comma-separated list of addresses of DNS resolver to use for the test")
protoFlag := flag.String("proto", "tcp,udp", "Comma-separated list of the protocols to test. Muse be \"tcp\", \"udp\", or a combination of them")
collectorFlag := flag.String("report-to", "", "URL to send JSON error reports to")
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps name the variable to match the flag, for clarity:

Suggested change
collectorFlag := flag.String("report-to", "", "URL to send JSON error reports to")
reportToFlag := flag.String("report-to", "", "URL to send JSON error reports to")

@@ -150,6 +184,13 @@ func main() {
if err != nil {
log.Fatalf("Failed to output JSON: %v", err)
}
// Send error report to collector if specified
if !success && *collectorFlag != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add flags for success and failure sampling rates, like NEL: https://www.w3.org/TR/network-error-logging/#sampling-rates

Perhaps -report-success-rate and -report-failure-rate, so they all start with -report-.

Perhaps set defaults to 0.1 and 1.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed on new PR

daniellacosse and others added 5 commits October 20, 2023 12:20
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.14.0 to 0.17.0.
- [Commits](golang/net@v0.14.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.14.0 to 0.17.0.
- [Commits](golang/net@v0.14.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Expose host and port on mobileproxy

* Update README to use new functions
@amircybersec
Copy link
Contributor Author

@fortuna I made a new branch/PR that applied all the comments discussed here. I messed up this branch and it was not worth spending time to fix it :|

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.

4 participants