Skip to content

Commit

Permalink
Merge pull request #57 from grafana/sync-grafana-agent-2024-03-21-1
Browse files Browse the repository at this point in the history
Sync grafana/agent:main with grafana/alloy:main (2024-03-21)
  • Loading branch information
rfratto authored Mar 21, 2024
2 parents 8d4de4c + d669bdb commit ce4699f
Show file tree
Hide file tree
Showing 45 changed files with 761 additions and 620 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ Main (unreleased)

- Update gcp_exporter to a newer version with a patch for incorrect delta histograms (@kgeckhart)

- Fix an issue where the default values of some component's arguments change
whenever that argument is explicitly configured. This issue only affected a
small subset of arguments across 15 components. (@erikbaranowski, @rfratto)

### Other changes

- Clustering for Grafana Agent in Flow mode has graduated from beta to stable.
Expand Down Expand Up @@ -108,7 +112,9 @@ v0.40.0 (2024-02-27)

### Deprecations

- Module components have been deprecated in favor of import and declare configuration blocks. These deprecated components will be removed in the next release. (@wildum)
- Module components have been deprecated in favor of import and declare configuration blocks. These deprecated components will be removed in a future release. (@wildum)

- `prometheus.exporter.vsphere` has been deprecated in favor of `otelcol.receiver.vcenter`. This deprecated component will be removed in a future release. (@rfratto)

### Features

Expand Down
4 changes: 2 additions & 2 deletions docs/sources/reference/components/loki.source.kubernetes.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ inside a `client` block.
[authorization]: #authorization-block
[oauth2]: #oauth2-block
[tls_config]: #tls_config-block
[clustering]: #clustering-beta
[clustering]: #clustering-block

### client block

Expand Down Expand Up @@ -133,7 +133,7 @@ Name | Type | Description

{{< docs/shared lookup="reference/components/tls-config-block.md" source="alloy" version="<ALLOY_VERSION>" >}}

### clustering (beta)
### clustering block

Name | Type | Description | Default | Required
----------|--------|-----------------------------------------------------|---------|---------
Expand Down
4 changes: 2 additions & 2 deletions docs/sources/reference/components/loki.source.podlogs.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ For example, `client > basic_auth` refers to a `basic_auth` block defined inside
[tls_config]: #tls_config-block
[selector]: #selector-block
[match_expression]: #match_expression-block
[clustering]: #clustering-beta
[clustering]: #clustering-block

### client block

Expand Down Expand Up @@ -225,7 +225,7 @@ The `operator` argument must be one of the following strings:
Both `selector` and `namespace_selector` can make use of multiple
`match_expression` inner blocks which are treated as AND clauses.

### clustering (beta)
### clustering block

Name | Type | Description | Default | Required
----------|--------|-----------------------------------------------------|---------|---------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ inside a `client` block.
[match_expression]: #match_expression-block
[rule]: #rule-block
[scrape]: #scrape-block
[clustering]: #clustering-beta
[clustering]: #clustering-block

### client block

Expand Down Expand Up @@ -159,7 +159,7 @@ The `operator` argument must be one of the following strings:

If there are multiple `match_expressions` blocks inside of a `selector` block, they are combined together with AND clauses.

### clustering (beta)
### clustering block

Name | Type | Description | Default | Required
---- | ---- | ----------- | ------- | --------
Expand Down
4 changes: 2 additions & 2 deletions docs/sources/reference/components/prometheus.scrape.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ an `oauth2` block.
[authorization]: #authorization-block
[oauth2]: #oauth2-block
[tls_config]: #tls_config-block
[clustering]: #clustering-beta
[clustering]: #clustering-block

### basic_auth block

Expand All @@ -131,7 +131,7 @@ an `oauth2` block.

{{< docs/shared lookup="reference/components/tls-config-block.md" source="alloy" version="<ALLOY_VERSION>" >}}

### clustering (beta)
### clustering block

Name | Type | Description | Default | Required
---- | ---- | ----------- | ------- | --------
Expand Down
4 changes: 2 additions & 2 deletions docs/sources/reference/components/pyroscope.scrape.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ the defaults documented in [profile.mutex][] will be used.
[profile.godeltaprof_block]: #profilegodeltaprof_block-block
[profile.custom]: #profilecustom-block
[pprof]: https://github.com/google/pprof/blob/main/doc/README.md
[clustering]: #clustering-beta
[clustering]: #clustering-block

[fgprof]: https://github.com/felixge/fgprof
[godeltaprof]: https://github.com/grafana/pyroscope-go/tree/main/godeltaprof
Expand Down Expand Up @@ -384,7 +384,7 @@ Name | Type | Description | Default | Required
When the `delta` argument is `true`, a `seconds` query parameter is
automatically added to requests. The `seconds` used will be equal to `scrape_interval - 1`.

### clustering (beta)
### clustering block

Name | Type | Description | Default | Required
---- | ---- | ----------- | ------- | --------
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,6 @@ require (
github.com/open-telemetry/opentelemetry-collector-contrib/processor/filterprocessor v0.87.0
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver v0.87.0
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/vcenterreceiver v0.87.0
github.com/prometheus/tsdb v0.10.0
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.42.0
golang.org/x/crypto/x509roots/fallback v0.0.0-20240208163226-62c9f1799c91
k8s.io/apimachinery v0.28.3
Expand Down
3 changes: 0 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,6 @@ github.com/dgryski/go-metro v0.0.0-20180109044635-280f6062b5bc h1:8WFBn63wegobsY
github.com/dgryski/go-metro v0.0.0-20180109044635-280f6062b5bc/go.mod h1:c9O8+fpSOX1DM8cPNSkX/qsBWdkD4yd2dpciOWQjpBw=
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78=
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
github.com/digitalocean/godo v1.1.1/go.mod h1:h6faOIcZ8lWIwNQ+DN7b3CgX4Kwby5T+nbpNqkUIozU=
github.com/digitalocean/godo v1.10.0/go.mod h1:h6faOIcZ8lWIwNQ+DN7b3CgX4Kwby5T+nbpNqkUIozU=
github.com/digitalocean/godo v1.104.1 h1:SZNxjAsskM/su0YW9P8Wx3gU0W1Z13b6tZlYNpl5BnA=
Expand Down Expand Up @@ -2006,8 +2005,6 @@ github.com/prometheus/snmp_exporter v0.24.1/go.mod h1:j6uIGkdR0DXvKn7HJtSkeDj//U
github.com/prometheus/statsd_exporter v0.22.7/go.mod h1:N/TevpjkIh9ccs6nuzY3jQn9dFqnUakOjnEuMPJJJnI=
github.com/prometheus/statsd_exporter v0.22.8 h1:Qo2D9ZzaQG+id9i5NYNGmbf1aa/KxKbB9aKfMS+Yib0=
github.com/prometheus/statsd_exporter v0.22.8/go.mod h1:/DzwbTEaFTE0Ojz5PqcSk6+PFHOPWGxdXVr6yC8eFOM=
github.com/prometheus/tsdb v0.10.0 h1:If5rVCMTp6W2SiRAQFlbpJNgVlgMEd+U2GZckwK38ic=
github.com/prometheus/tsdb v0.10.0/go.mod h1:oi49uRhEe9dPUTlS3JRZOwJuVi6tmh10QSgwXEyGCt4=
github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 h1:N/ElC8H3+5XpJzTSTfLsJV/mx9Q9g7kxmchpfZyxgzM=
Expand Down
176 changes: 176 additions & 0 deletions internal/component/all/all_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package all

import (
"fmt"
"reflect"
"testing"

"github.com/grafana/agent/internal/component"
"github.com/grafana/river"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestSetDefault_NoPointerReuse ensures that calls to SetDefault do not re-use
// pointers. The test iterates through all registered components, and then
// recursively traverses through its Arguments type to guarantee that no two
// calls to SetDefault result in pointer reuse.
//
// Nested types that also implement river.Defaulter are also checked.
func TestSetDefault_NoPointerReuse(t *testing.T) {
allComponents := component.AllNames()
for _, componentName := range allComponents {
reg, ok := component.Get(componentName)
require.True(t, ok, "Expected component %q to exist", componentName)

t.Run(reg.Name, func(t *testing.T) {
testNoReusePointer(t, reg)
})
}
}

func testNoReusePointer(t *testing.T, reg component.Registration) {
t.Helper()

var (
args1 = reg.CloneArguments()
args2 = reg.CloneArguments()
)

if args1, ok := args1.(river.Defaulter); ok {
args1.SetToDefault()
}
if args2, ok := args2.(river.Defaulter); ok {
args2.SetToDefault()
}

rv1, rv2 := reflect.ValueOf(args1), reflect.ValueOf(args2)
ty := rv1.Type().Elem()

// Edge case: if the component's arguments type is an empty struct, skip.
// Not skipping causes the test to fail, due to an optimization in
// reflect.New where initializing the same zero-length object results in the
// same pointer.
if rv1.Elem().NumField() == 0 {
return
}

if path, shared := sharePointer(rv1, rv2); shared {
fullPath := fmt.Sprintf("%s.%s.%s", ty.PkgPath(), ty.Name(), path)

assert.Fail(t,
fmt.Sprintf("Detected SetToDefault pointer reuse at %s", fullPath),
"Types implementing river.Defaulter must not reuse pointers across multiple calls. Doing so leads to default values being changed when unmarshaling configuration files. If you're seeing this error, check the path above and ensure that copies are being made of any pointers in all instances of SetToDefault calls where that field is used.",
)
}
}

func sharePointer(a, b reflect.Value) (string, bool) {
// We want to recursively check a and b, so if they're nil they need to be
// initialized to see if any of their inner values have shared pointers after
// being initialized with defaults.
initValue(a)
initValue(b)

// From the documentation of reflect.Value.Pointer, values of chan, func,
// map, pointer, slice, and unsafe pointer are all pointer values.
//
// Additionally, we want to recurse into values (even if they don't have
// addresses) to see if there's shared pointers inside of them.
switch a.Kind() {
case reflect.Chan, reflect.Func, reflect.UnsafePointer:
return "", a.Pointer() == b.Pointer()

case reflect.Map:
if pointersMatch(a, b) {
return "", true
}

iter := a.MapRange()
for iter.Next() {
aValue, bValue := iter.Value(), b.MapIndex(iter.Key())
if !bValue.IsValid() {
continue
}
if path, shared := sharePointer(aValue, bValue); shared {
return path, true
}
}
return "", false

case reflect.Pointer:
if pointersMatch(a, b) {
return "", true
} else {
// Recursively navigate inside of the pointer.
return sharePointer(a.Elem(), b.Elem())
}

case reflect.Interface:
if a.UnsafeAddr() == b.UnsafeAddr() {
return "", true
}
return sharePointer(a.Elem(), b.Elem())

case reflect.Slice:
if pointersMatch(a, b) {
// If the slices are preallocated immutable pointers such as []string{}, we can ignore
if a.Len() == 0 && a.Cap() == 0 && b.Len() == 0 && b.Cap() == 0 {
return "", false
}
return "", true
}

size := min(a.Len(), b.Len())
for i := 0; i < size; i++ {
if path, shared := sharePointer(a.Index(i), b.Index(i)); shared {
return path, true
}
}
return "", false
}

// Recurse into non-pointer types.
switch a.Kind() {
case reflect.Array:
for i := 0; i < a.Len(); i++ {
if path, shared := sharePointer(a.Index(i), b.Index(i)); shared {
return path, true
}
}
return "", false

case reflect.Struct:
// Check to make sure there are no shared pointers between args1 and args2.
for i := 0; i < a.NumField(); i++ {
if path, shared := sharePointer(a.Field(i), b.Field(i)); shared {
fullPath := a.Type().Field(i).Name
if path != "" {
fullPath += "." + path
}
return fullPath, true
}
}
return "", false
}

return "", false
}

func pointersMatch(a, b reflect.Value) bool {
if a.IsNil() || b.IsNil() {
return false
}
return a.Pointer() == b.Pointer()
}

// initValue initializes nil pointers. If the nil pointer implements
// river.Defaulter, it is also set to default values.
func initValue(rv reflect.Value) {
if rv.Kind() == reflect.Pointer && rv.IsNil() {
rv.Set(reflect.New(rv.Type().Elem()))
if defaulter, ok := rv.Interface().(river.Defaulter); ok {
defaulter.SetToDefault()
}
}
}
16 changes: 7 additions & 9 deletions internal/component/discovery/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,15 @@ type Arguments struct {
Namespaces []string `river:"namespaces,attr,optional"`
}

// DefaultConfig holds defaults for SDConfig.
var DefaultConfig = Arguments{
URL: config.URL{
URL: defaultKubeletURL,
},
HTTPClientConfig: config.DefaultHTTPClientConfig,
}

// SetToDefault implements river.Defaulter.
func (args *Arguments) SetToDefault() {
*args = DefaultConfig
cloneDefaultKubeletUrl := *defaultKubeletURL
*args = Arguments{
URL: config.URL{
URL: &cloneDefaultKubeletUrl,
},
HTTPClientConfig: config.DefaultHTTPClientConfig,
}
}

// Validate implements river.Validator.
Expand Down
12 changes: 9 additions & 3 deletions internal/component/discovery/kubelet/kubelet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ func TestPodDeletion(t *testing.T) {
Items: []v1.Pod{pod2},
}

kubeletDiscovery, err := NewKubeletDiscovery(DefaultConfig)
var args Arguments
args.SetToDefault()
kubeletDiscovery, err := NewKubeletDiscovery(args)
require.NoError(t, err)

_, err = kubeletDiscovery.refresh(podList1)
Expand Down Expand Up @@ -100,7 +102,9 @@ func TestDiscoveryPodWithoutPod(t *testing.T) {
Items: []v1.Pod{pod1, pod2},
}

kubeletDiscovery, err := NewKubeletDiscovery(DefaultConfig)
var args Arguments
args.SetToDefault()
kubeletDiscovery, err := NewKubeletDiscovery(args)
require.NoError(t, err)

_, err = kubeletDiscovery.refresh(podList1)
Expand All @@ -109,7 +113,9 @@ func TestDiscoveryPodWithoutPod(t *testing.T) {
}

func TestWithDefaultKubeletHost(t *testing.T) {
kubeletDiscovery, err := NewKubeletDiscovery(DefaultConfig)
var args Arguments
args.SetToDefault()
kubeletDiscovery, err := NewKubeletDiscovery(args)
require.NoError(t, err)
require.Equal(t, "https://localhost:10250/pods", kubeletDiscovery.url)
}
Expand Down
Loading

0 comments on commit ce4699f

Please sign in to comment.