From 0fabd24f3fff3314ab0baf196620e7b7fe233cb0 Mon Sep 17 00:00:00 2001
From: Johannes Tax <johannes.tax@grafana.com>
Date: Wed, 29 May 2024 10:56:18 +0200
Subject: [PATCH] PR comments

---
 detectors/azure/vm/vm.go      | 35 ++++++++++++++---------------------
 detectors/azure/vm/vm_test.go | 33 ++++++++++++++++-----------------
 2 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/detectors/azure/vm/vm.go b/detectors/azure/vm/vm.go
index 47896ed3b13..b45f9b16589 100644
--- a/detectors/azure/vm/vm.go
+++ b/detectors/azure/vm/vm.go
@@ -14,12 +14,16 @@ import (
 	semconv "go.opentelemetry.io/otel/semconv/v1.24.0"
 )
 
+const (
+	defaultAzureVmMetadataEndpoint = "http://169.254.169.254/metadata/instance/compute?api-version=2021-12-13&format=json"
+)
+
 type config struct {
-	client Client
+	endpoint string
 }
 
 func newConfig(options ...Option) config {
-	c := config{&azureInstanceMetadataClient{}}
+	c := config{defaultAzureVmMetadataEndpoint}
 	for _, option := range options {
 		c = option.apply(c)
 	}
@@ -38,21 +42,17 @@ func (fn optionFunc) apply(c config) config {
 	return fn(c)
 }
 
-// WithClient sets the client for obtaining a Azure instance metadata JSON.
-func WithClient(t Client) Option {
+// WithEndpoint sets the endpoint for obtaining a Azure instance metadata JSON.
+func WithEndpoint(e string) Option {
 	return optionFunc(func(c config) config {
-		c.client = t
+		c.endpoint = e
 
 		return c
 	})
 }
 
-func (cfg config) getClient() Client {
-	return cfg.client
-}
-
 type resourceDetector struct {
-	client Client
+	endpoint string
 }
 
 type vmMetadata struct {
@@ -68,12 +68,12 @@ type vmMetadata struct {
 // New returns a [resource.Detector] that will detect Azure VM resources.
 func New(opts ...Option) resource.Detector {
 	c := newConfig(opts...)
-	return &resourceDetector{c.getClient()}
+	return &resourceDetector{c.endpoint}
 }
 
 // Detect detects associated resources when running on an Azure VM.
 func (detector *resourceDetector) Detect(ctx context.Context) (*resource.Resource, error) {
-	jsonMetadata, err := detector.client.GetJSONMetadata()
+	jsonMetadata, err := detector.getJSONMetadata()
 	if err != nil {
 		return nil, err
 	}
@@ -114,19 +114,12 @@ func (detector *resourceDetector) Detect(ctx context.Context) (*resource.Resourc
 	return resource.NewWithAttributes(semconv.SchemaURL, attributes...), nil
 }
 
-// Client is an interface that allows mocking for testing.
-type Client interface {
-	GetJSONMetadata() ([]byte, error)
-}
-
-type azureInstanceMetadataClient struct{}
-
-func (c *azureInstanceMetadataClient) GetJSONMetadata() ([]byte, error) {
+func (detector *resourceDetector) getJSONMetadata() ([]byte, error) {
 	PTransport := &http.Transport{Proxy: nil}
 
 	client := http.Client{Transport: PTransport}
 
-	req, err := http.NewRequest("GET", "http://169.254.169.254/metadata/instance/compute?api-version=2021-12-13&format=json", nil)
+	req, err := http.NewRequest("GET", detector.endpoint, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/detectors/azure/vm/vm_test.go b/detectors/azure/vm/vm_test.go
index eb631ede2a1..216694c3927 100644
--- a/detectors/azure/vm/vm_test.go
+++ b/detectors/azure/vm/vm_test.go
@@ -5,7 +5,9 @@ package vm
 
 import (
 	"context"
-	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -18,7 +20,7 @@ import (
 func TestDetect(t *testing.T) {
 	type input struct {
 		jsonMetadata string
-		err          error
+		statusCode   int
 	}
 	type expected struct {
 		resource *resource.Resource
@@ -41,7 +43,7 @@ func TestDetect(t *testing.T) {
 					"osType": "linux",
 					"version": "6.5.0-26-generic"
 				}`,
-				err: nil,
+				statusCode: http.StatusOK, 
 			},
 			expected: expected{
 				resource: resource.NewWithAttributes(semconv.SchemaURL, []attribute.KeyValue{
@@ -61,7 +63,7 @@ func TestDetect(t *testing.T) {
 		{
 			input: input{
 				jsonMetadata: `{`,
-				err:          nil,
+				statusCode: http.StatusOK, 
 			},
 			expected: expected{
 				resource: nil,
@@ -71,7 +73,7 @@ func TestDetect(t *testing.T) {
 		{
 			input: input{
 				jsonMetadata: "",
-				err:          errors.New("cannot get metadata"),
+				statusCode: http.StatusNotFound, 
 			},
 			expected: expected{
 				resource: nil,
@@ -81,10 +83,16 @@ func TestDetect(t *testing.T) {
 	}
 
 	for _, tCase := range testTable {
-		detector := New(WithClient(&mockClient{
-			jsonMetadata: []byte(tCase.input.jsonMetadata),
-			err:          tCase.input.err,
+		svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+
+			if r.Header.Get("Metadata") == "True" {
+				fmt.Fprintf(w, tCase.input.jsonMetadata)
+			}
 		}))
+		defer svr.Close()
+
+		detector := New(WithEndpoint(svr.URL))
 
 		azureResource, err := detector.Detect(context.Background())
 
@@ -92,12 +100,3 @@ func TestDetect(t *testing.T) {
 		assert.Equal(t, tCase.expected.resource, azureResource)
 	}
 }
-
-type mockClient struct {
-	jsonMetadata []byte
-	err          error
-}
-
-func (c *mockClient) GetJSONMetadata() ([]byte, error) {
-	return c.jsonMetadata, c.err
-}