Skip to content

Commit

Permalink
[pkg/stanza] when the 'jsonArrayParserFeatureGate' is disabled, the f…
Browse files Browse the repository at this point in the history
…unc 'Build' returns an error (#32501)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
bug: 
- collector launch fail with '--feature-gates=logs.jsonParserArray'

how to fix:
1. `func init()` always register operator to `operator.DefaultRegistry`.
2. when the `jsonArrayParserFeatureGate` is disabled, the func `Build`
returns an error.

**Link to tracking Issue:** <Issue number if applicable>

#32313

---------

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
  • Loading branch information
li-zeyuan and djaglowski authored Apr 19, 2024
1 parent 6d8b65d commit 772cbc6
Show file tree
Hide file tree
Showing 16 changed files with 104 additions and 4 deletions.
27 changes: 27 additions & 0 deletions .chloggen/fix-jsonarrayparser-build.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: pkg/stanza

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix race condition which prevented `jsonArrayParserFeatureGate` from working correctly.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [32313]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
3 changes: 3 additions & 0 deletions pkg/stanza/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/jpillora/backoff v1.0.0
github.com/json-iterator/go v1.1.12
github.com/open-telemetry/opentelemetry-collector-contrib/extension/storage v0.98.0
github.com/open-telemetry/opentelemetry-collector-contrib/internal/common v0.98.0
github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.98.0
github.com/stretchr/testify v1.9.0
github.com/valyala/fastjson v1.6.4
Expand Down Expand Up @@ -78,6 +79,8 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/extension/stor

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal => ../../internal/coreinternal

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common

retract (
v0.76.2
v0.76.1
Expand Down
9 changes: 6 additions & 3 deletions pkg/stanza/operator/parser/jsonarray/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package jsonarray // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/operator/parser/jsonarray"

import (
"fmt"
"strings"

"github.com/valyala/fastjson"
Expand All @@ -26,9 +27,7 @@ var jsonArrayParserFeatureGate = featuregate.GlobalRegistry().MustRegister(
)

func init() {
if jsonArrayParserFeatureGate.IsEnabled() {
operator.Register(operatorType, func() operator.Builder { return NewConfig() })
}
operator.Register(operatorType, func() operator.Builder { return NewConfig() })
}

// NewConfig creates a new json array parser config with default values
Expand All @@ -51,6 +50,10 @@ type Config struct {

// Build will build a json array parser operator.
func (c Config) Build(logger *zap.SugaredLogger) (operator.Operator, error) {
if !jsonArrayParserFeatureGate.IsEnabled() {
return nil, fmt.Errorf("%s operator disabled", operatorType)
}

parserOperator, err := c.ParserConfig.Build(logger)
if err != nil {
return nil, err
Expand Down
34 changes: 33 additions & 1 deletion pkg/stanza/operator/parser/jsonarray/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@ import (
"path/filepath"
"testing"

"github.com/stretchr/testify/require"

commontestutil "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/entry"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/operator"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/operator/operatortest"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/testutil"
)

func TestConfig(t *testing.T) {
// Manually adding operator to the Registry as its behind a feature gate
// Manually adding operator to the Registry as it's behind a feature gate
operator.Register(operatorType, func() operator.Builder { return NewConfig() })

operatortest.ConfigUnmarshalTests{
Expand Down Expand Up @@ -65,3 +69,31 @@ func TestConfig(t *testing.T) {
},
}.Run(t)
}

func TestBuildWithFeatureGate(t *testing.T) {
cases := []struct {
name string
isFeatureGateEnable bool
onErr string
}{
{"jsonarray_enabled", true, ""},
{"jsonarray_disabled", false, "operator disabled"},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
if c.isFeatureGateEnable {
defer commontestutil.SetFeatureGateForTest(t, jsonArrayParserFeatureGate, true)()
}

buildFunc, ok := operator.Lookup(operatorType)
require.True(t, ok)

_, err := buildFunc().Build(testutil.Logger(t))
if err != nil {
require.Contains(t, err.Error(), c.onErr)
}
})
}

}
13 changes: 13 additions & 0 deletions pkg/stanza/operator/parser/jsonarray/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,24 @@ import (

"github.com/stretchr/testify/require"

commontestutil "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/entry"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/operator"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/testutil"
)

func newTestParser(t *testing.T) *Parser {
defer commontestutil.SetFeatureGateForTest(t, jsonArrayParserFeatureGate, true)()

cfg := NewConfigWithID("test")
op, err := cfg.Build(testutil.Logger(t))
require.NoError(t, err)
return op.(*Parser)
}

func TestParserBuildFailure(t *testing.T) {
defer commontestutil.SetFeatureGateForTest(t, jsonArrayParserFeatureGate, true)()

cfg := NewConfigWithID("test")
cfg.OnError = "invalid_on_error"
_, err := cfg.Build(testutil.Logger(t))
Expand All @@ -37,6 +42,8 @@ func TestParserInvalidType(t *testing.T) {
}

func TestParserByteFailureHeadersMismatch(t *testing.T) {
defer commontestutil.SetFeatureGateForTest(t, jsonArrayParserFeatureGate, true)()

cfg := NewConfigWithID("test")
cfg.Header = "name,sev,msg"
op, err := cfg.Build(testutil.Logger(t))
Expand All @@ -48,6 +55,8 @@ func TestParserByteFailureHeadersMismatch(t *testing.T) {
}

func TestParserJarray(t *testing.T) {
defer commontestutil.SetFeatureGateForTest(t, jsonArrayParserFeatureGate, true)()

cases := []struct {
name string
configure func(*Config)
Expand Down Expand Up @@ -268,6 +277,8 @@ func TestParserJarray(t *testing.T) {
}

func TestParserJarrayMultiline(t *testing.T) {
defer commontestutil.SetFeatureGateForTest(t, jsonArrayParserFeatureGate, true)()

cases := []struct {
name string
input string
Expand Down Expand Up @@ -374,6 +385,8 @@ dd","eeee"]`,
}

func TestBuildParserJarray(t *testing.T) {
defer commontestutil.SetFeatureGateForTest(t, jsonArrayParserFeatureGate, true)()

newBasicParser := func() *Config {
cfg := NewConfigWithID("test")
cfg.OutputIDs = []string{"test"}
Expand Down
2 changes: 2 additions & 0 deletions processor/logstransformprocessor/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/azureeventhubreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/share
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/azure => ../../pkg/translator/azure

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/filelogreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/journaldreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/namedpipereceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/otlpjsonfilereceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/sqlqueryreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza =>
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/sqlquery => ../../internal/sqlquery

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/syslogreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/tcplogreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/udplogreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common
2 changes: 2 additions & 0 deletions receiver/windowseventlogreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/common => ../../internal/common

0 comments on commit 772cbc6

Please sign in to comment.