Repository: open-telemetry/opamp-go
Pull request #116 (2022-07-29 15:39:02)

Require that instance_uid is ULID
Implements spec change https://github.com/open-telemetry/opamp-spec/pull/116

codecov[bot]: # [Codecov](https://codecov.io/gh/open-telemetry/opamp-go/pull/116?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) Report
> Merging [#116](https://codecov.io/gh/open-telemetry/opamp-go/pull/116?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) (2f74193) into [main](https://codecov.io/gh/open-telemetry/opamp-go/commit/3d4cbdc975d341c1765050a9a59882ef89dda006?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) (3d4cbdc) will **increase** coverage by `1.27%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
+ Coverage   74.89%   76.16%   +1.27%     
==========================================
  Files          22       22              
  Lines        1629     1632       +3     
==========================================
+ Hits         1220     1243      +23     
+ Misses        307      293      -14     
+ Partials      102       96       -6     
```


| [Impacted Files](https://codecov.io/gh/open-telemetry/opamp-go/pull/116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) | Coverage Δ | |
|---|---|---|
| [client/internal/sender.go](https://codecov.io/gh/open-telemetry/opamp-go/pull/116/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-Y2xpZW50L2ludGVybmFsL3NlbmRlci5nbw==) | `88.00% <100.00%> (+1.63%)` | :arrow_up: |
| [client/internal/wssender.go](https://codecov.io/gh/open-telemetry/opamp-go/pull/116/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-Y2xpZW50L2ludGVybmFsL3dzc2VuZGVyLmdv) | `81.81% <0.00%> (-9.10%)` | :arrow_down: |
| [client/internal/clientcommon.go](https://codecov.io/gh/open-telemetry/opamp-go/pull/116/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-Y2xpZW50L2ludGVybmFsL2NsaWVudGNvbW1vbi5nbw==) | `81.00% <0.00%> (+1.50%)` | :arrow_up: |
| [client/internal/receivedprocessor.go](https://codecov.io/gh/open-telemetry/opamp-go/pull/116/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-Y2xpZW50L2ludGVybmFsL3JlY2VpdmVkcHJvY2Vzc29yLmdv) | `84.28% <0.00%> (+15.00%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/open-telemetry/opamp-go/pull/116?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/open-telemetry/opamp-go/pull/116?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry). Last update [3d4cbdc...2f74193](https://codecov.io/gh/open-telemetry/opamp-go/pull/116?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry).


Patch:
diff --git a/client/internal/sender.go b/client/internal/sender.go
index 28a283d..f418b4c 100644
--- a/client/internal/sender.go
+++ b/client/internal/sender.go
@@ -3,6 +3,7 @@ package internal
 import (
 	"errors"
 
+	"github.com/oklog/ulid/v2"
 	"github.com/open-telemetry/opamp-go/protobufs"
 )
 
@@ -69,6 +70,10 @@ func (h *SenderCommon) SetInstanceUid(instanceUid string) error {
 		return errors.New("cannot set instance uid to empty value")
 	}
 
+	if _, err := ulid.ParseStrict(instanceUid); err != nil {
+		return err
+	}
+
 	h.nextMessage.Update(
 		func(msg *protobufs.AgentToServer) {
 			msg.InstanceUid = instanceUid


Test patch:
diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go
index 226b0e6..1af68c6 100644
--- a/client/clientimpl_test.go
+++ b/client/clientimpl_test.go
@@ -117,6 +117,17 @@ func TestConnectNoServer(t *testing.T) {
 	})
 }
 
+func TestInvalidInstanceId(t *testing.T) {
+	testClients(t, func(t *testing.T, client OpAMPClient) {
+		settings := createNoServerSettings()
+		prepareClient(t, &settings, client)
+		settings.InstanceUid = "invalidid"
+
+		err := client.Start(context.Background(), settings)
+		assert.Error(t, err)
+	})
+}
+
 func TestOnConnectFail(t *testing.T) {
 	testClients(t, func(t *testing.T, client OpAMPClient) {
 		var connectErr atomic.Value