Repository: beatlabs/patron
Pull request #306 (2020-12-04 16:20:53)

Set PATRON_COMPRESSION_DEFLATE_LEVEL setting
<!--
Thanks for taking precious time for making a PR.

Before creating a pull request, please make sure:
- Your PR solves one problem for which a issue exist and a solution has been discussed
- You have read the guide for contributing
  - See https://github.com/beatlabs/patron/blob/master/README.md#how-to-contribute
- You signed all your commits (otherwise we won't be able to merge the PR)
  - See https://github.com/beatlabs/patron/blob/master/SIGNYOURWORK.md
- You added unit tests for the new functionality
- You mention in the PR description which issue it is addressing, e.g. "Resolves #123"
-->

## Which problem is this PR solving?

Issue https://github.com/beatlabs/patron/issues/302

## Short description of the changes

Set the new setting the same style as read/write timeouts.


azarakovskiy: @mantzas it makes sense to decrease the default level, although
1. It is in a different scope than the issue I am solving here.
2. There should be an investigation of what value is better as an average because different use cases /services require different settings.

I can easily go to `6` because it's Golang default, hence justified by that, but for any other better value, I would keep it outside of the scope here.
codecov[bot]: # [Codecov](https://codecov.io/gh/beatlabs/patron/pull/306?src=pr&el=h1) Report
> Merging [#306](https://codecov.io/gh/beatlabs/patron/pull/306?src=pr&el=desc) (09c3584) into [master](https://codecov.io/gh/beatlabs/patron/commit/c0556559f4a96c5cc1aa7d2c7fe651e58715e540?el=desc) (c055655) will **increase** coverage by `0.05%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/beatlabs/patron/pull/306/graphs/tree.svg?width=650&height=150&src=pr&token=sxY15rXW1X)](https://codecov.io/gh/beatlabs/patron/pull/306?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #306      +/-   ##
==========================================
+ Coverage   72.23%   72.29%   +0.05%     
==========================================
  Files          61       61              
  Lines        3288     3295       +7     
==========================================
+ Hits         2375     2382       +7     
  Misses        820      820              
  Partials       93       93              
```


| [Impacted Files](https://codecov.io/gh/beatlabs/patron/pull/306?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [service.go](https://codecov.io/gh/beatlabs/patron/pull/306/diff?src=pr&el=tree#diff-c2VydmljZS5nbw==) | `89.55% <100.00%> (+0.37%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/beatlabs/patron/pull/306?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/beatlabs/patron/pull/306?src=pr&el=footer). Last update [c055655...09c3584](https://codecov.io/gh/beatlabs/patron/pull/306?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

mantzas: > @mantzas it makes sense to decrease the default level, although
> 
> 1. It is in a different scope than the issue I am solving here.
> 2. There should be an investigation of what value is better as an average because different use cases /services require different settings.
> 
> I can easily go to `6` because it's Golang default, hence justified by that, but for any other better value, I would keep it outside of the scope here.

If 6 is the Go standard then let's use 6. No need to do anything more than that.
azarakovskiy: @mantzas done, set to 6

Patch:
diff --git a/component/http/component.go b/component/http/component.go
index edd9c9bc6..85744439a 100644
--- a/component/http/component.go
+++ b/component/http/component.go
@@ -19,7 +19,7 @@ const (
 	httpWriteTimeout    = 10 * time.Second
 	httpIdleTimeout     = 120 * time.Second
 	shutdownGracePeriod = 5 * time.Second
-	deflateLevel        = 8
+	deflateLevel        = 6
 )
 
 var (
diff --git a/service.go b/service.go
index d6dd30ec6..7d60ad962 100644
--- a/service.go
+++ b/service.go
@@ -114,6 +114,16 @@ func (s *service) createHTTPComponent() (Component, error) {
 		log.Infof("setting up default HTTP write timeout %s", httpWriteTimeout)
 	}
 
+	deflateLevel, ok := os.LookupEnv("PATRON_COMPRESSION_DEFLATE_LEVEL")
+	if ok {
+		deflateLevelInt, err := strconv.Atoi(deflateLevel)
+		if err != nil {
+			return nil, fmt.Errorf("env var for HTTP deflate level is not valid: %w", err)
+		}
+		b.WithDeflateLevel(deflateLevelInt)
+		log.Infof("setting up default HTTP deflate level  %s", deflateLevel)
+	}
+
 	if s.acf != nil {
 		b.WithAliveCheckFunc(s.acf)
 	}


Test patch:
diff --git a/service_test.go b/service_test.go
index 0df1d9f73..08fd5ef8e 100644
--- a/service_test.go
+++ b/service_test.go
@@ -131,6 +131,8 @@ func TestServer_Run_Shutdown(t *testing.T) {
 	}
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
+			defer os.Clearenv()
+
 			err := os.Setenv("PATRON_HTTP_DEFAULT_PORT", getRandomPort())
 			assert.NoError(t, err)
 			svc, err := New("test", "", TextLogger())
@@ -159,6 +161,8 @@ func TestServer_SetupTracing(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			defer os.Clearenv()
+
 			if tt.host != "" {
 				err := os.Setenv("PATRON_JAEGER_AGENT_HOST", tt.host)
 				assert.NoError(t, err)
@@ -198,6 +202,8 @@ func TestBuild_FailingConditions(t *testing.T) {
 
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
+			defer os.Clearenv()
+
 			if tt.samplerParam != "" {
 				err := os.Setenv("PATRON_JAEGER_SAMPLER_PARAM", tt.samplerParam)
 				assert.NoError(t, err)
@@ -215,11 +221,6 @@ func TestBuild_FailingConditions(t *testing.T) {
 				assert.NotNil(t, svc)
 			}
 		})
-		err := os.Unsetenv("PATRON_JAEGER_SAMPLER_PARAM")
-		require.NoError(t, err)
-
-		err = os.Unsetenv("PATRON_HTTP_DEFAULT_PORT")
-		require.NoError(t, err)
 	}
 }
 
@@ -241,6 +242,8 @@ func TestServer_SetupReadWriteTimeouts(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			defer os.Clearenv()
+
 			if tt.rt != "" {
 				err := os.Setenv("PATRON_HTTP_READ_TIMEOUT", tt.rt)
 				assert.NoError(t, err)
@@ -262,6 +265,44 @@ func TestServer_SetupReadWriteTimeouts(t *testing.T) {
 	}
 }
 
+func TestServer_SetupDeflateLevel(t *testing.T) {
+	tests := []struct {
+		name      string
+		component Component
+		ctx       context.Context
+		level     string
+		wantErr   bool
+	}{
+		{name: "success without setup compression deflate level", component: &testComponent{}, ctx: context.Background(), wantErr: false},
+		{name: "success with setup compression deflate level = -2", component: &testComponent{}, ctx: context.Background(), level: "-2", wantErr: false},
+		{name: "success with setup compression deflate level = 2", component: &testComponent{}, ctx: context.Background(), level: "2", wantErr: false},
+		{name: "success with setup compression deflate level = 6", component: &testComponent{}, ctx: context.Background(), level: "6", wantErr: false},
+		{name: "success with setup compression deflate level = 9", component: &testComponent{}, ctx: context.Background(), level: "9", wantErr: false},
+		{name: "failed with too small compression deflate level", component: &testComponent{}, ctx: context.Background(), level: "-3", wantErr: true},
+		{name: "failed with too big compression deflate level", component: &testComponent{}, ctx: context.Background(), level: "10", wantErr: true},
+		{name: "failed with invalid compression deflate level", component: &testComponent{}, ctx: context.Background(), level: "blah", wantErr: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defer os.Clearenv()
+
+			if tt.level != "" {
+				err := os.Setenv("PATRON_COMPRESSION_DEFLATE_LEVEL", tt.level)
+				assert.NoError(t, err)
+			}
+			svc, err := New("test", "", TextLogger())
+			require.NoError(t, err)
+
+			_, err = svc.WithComponents(tt.component, tt.component, tt.component).build()
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
 func getRandomPort() string {
 	rnd := 50000 + rand.Int63n(10000)
 	return strconv.FormatInt(rnd, 10)