Reporter package

[amircybersec]

@fortuna I refactored the send report functionality with random sampling into a separate reporter package to be imported when needed in other apps. Since I am using this logic in both outline-connectivity and outline-connectivity-app, it would be repetitive to have the code block in both places. I recommend dropping the other PR and get this PR merged first:

#112

I can then do another PR for refactored main.go for for both connectivity apps that uses this package for reporting.

[fortuna]

TODO: WriteCollector that takes a io.Writer

[fortuna]

Suggested change

	log.Printf("Error encoding JSON: %s\n", err)
	return err
	return fmt.Errorf("failed to marshal JSON: %w", err)

[fortuna]

We should remove all logging from this library.

Suggested change

var debugLog log.Logger = *log.New(io.Discard, "", 0)

[fortuna]

Suggested change

log.Printf("Error collecting report: %v", err)

[fortuna]

Pass the client to the RemoteReporter

Suggested change

var httpClient = &http.Client{}

[fortuna]

Suggested change

	collectorEndpoint *url.URL
	httpClient *http.Client
	collectorEndpoint *url.URL

[fortuna]

All public types and function must have comments.

[fortuna]

Suggested change

	type RetryCollector struct {
	// RetryCollector is a [Collector] that retries in case of [BadRequestErr].
	type RetryCollector struct {

[fortuna]

Suggested change

	type BadRequestErr struct {
	StatusCode int
	Message string
	}
	func (e BadRequestErr) Error() string {
	return e.Message
	}
	type BadRequestError {
	error
	}
	func (e BadRequestError) Unwrap() error {
	return e.error
	}

Then you can do:

  return BadRequestError{err}

[fortuna]

Suggested change

	if resp.StatusCode >= 400 {
	if 400 <= resp.StatusCode && resp.StatusCode < 500 {

[fortuna]

Suggested change

	return BadRequestErr{
	StatusCode: resp.StatusCode,
	Message: fmt.Sprintf("HTTP request failed with status code %d", resp.StatusCode),
	}
	return BadRequestErr{err}

[fortuna]

It seems the standard library uses the Err prefix for error constants and the Error suffix for type, so we should use BadRequestError here.

[fortuna]

Let's drop this for now, since the semantics can be confusing and it's not clear we will use it.

Suggested change

	// MultiCollector represents a collector that combines multiple collectors.
	type MultiCollector struct {
	collectors []Collector
	}
	// Collect implements [Collector] interface on [MultiCollector] type.
	// It collects the report using all the provided collectors in the [MultiCollector].
	// It returns an error if all collectors fail to collect the report.
	func (c *MultiCollector) Collect(ctx context.Context, report Report) error {
	success := false
	for i := range c.collectors {
	err := c.collectors[i].Collect(ctx, report)
	if err != nil {
	success = success || false
	} else {
	success = success || true
	}
	}
	if success {
	// At least one collector succeeded
	return nil
	}
	return errors.New("all collectors failed")
	}

[fortuna]

This should do exponential backoff: https://harish-bhattbhatt.medium.com/best-practices-for-retry-pattern-f29d47cd5117