Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve OpenTelemetry Collector JSON Marshalling #3286

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cb6600e
Improve OpenTelemetry Collector JSON Marshalling
iblancasa Sep 12, 2024
f242919
Add clarification
iblancasa Sep 19, 2024
daaab63
Merge branch 'main' into bug/3281
iblancasa Sep 19, 2024
4a08965
Merge branch 'main' into bug/3281
iblancasa Sep 23, 2024
3812d6e
Merge branch 'main' into bug/3281
iblancasa Sep 23, 2024
dfc89b2
Merge branch 'main' into bug/3281
iblancasa Sep 24, 2024
b9bce9a
Merge branch 'bug/3281' of github.com:iblancasa/opentelemetry-operato…
iblancasa Sep 26, 2024
bd41d8c
Apply changes requested in code review
iblancasa Sep 26, 2024
32264b6
Improve changelog
iblancasa Sep 26, 2024
9d2ed17
Merge branch 'main' into bug/3281
iblancasa Sep 26, 2024
4c05ee6
Merge branch 'main' into bug/3281
iblancasa Sep 30, 2024
8d63442
Merge branch 'main' into bug/3281
iblancasa Sep 30, 2024
bff339f
Merge branch 'main' into bug/3281
iblancasa Oct 1, 2024
11a2258
Merge branch 'main' into bug/3281
iblancasa Oct 1, 2024
6049ced
Merge branch 'bug/3281' of github.com:iblancasa/opentelemetry-operato…
iblancasa Oct 2, 2024
f3e8de0
Remove not needed copy
iblancasa Oct 2, 2024
5f67714
Merge branch 'main' into bug/3281
iblancasa Oct 2, 2024
a9131d3
Merge branch 'main' into bug/3281
iblancasa Oct 3, 2024
f86f7d5
Merge branch 'main' into bug/3281
iblancasa Oct 7, 2024
2e6916f
Merge branch 'bug/3281' of github.com:iblancasa/opentelemetry-operato…
iblancasa Oct 8, 2024
8820e14
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
iblancasa Oct 8, 2024
905b5d7
Merge branch 'main' into bug/3281
iblancasa Oct 9, 2024
7505d5a
Merge branch 'main' into bug/3281
iblancasa Oct 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .chloggen/3281-marshal-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# 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. collector, target allocator, auto-instrumentation, opamp, github action)
component: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix Configuration marshalling for v1beta1 OpenTelemetryCollector instances.

# One or more tracking issues related to the change
issues: [3281]

# (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: |
Previously, the v1beta1 OpenTelemetryCollector CR's config field was incorrectly
marshalled, potentially causing issues when storing in etcd or retrieving via kubectl.
This fix ensures that the config is properly marshalled, maintaining the correct
structure and data types. For example, a config with nested maps and arrays will now
be correctly preserved when stored and retrieved, instead of being flattened or
losing structure.

As an example, configuration like this:

receivers:
otlp:
protocols:
grpc:
http:

Would be retrieved from the Kubernetes cluster as:

receivers:
otlp:
protocols:
jaronoff97 marked this conversation as resolved.
Show resolved Hide resolved

The specified protocols were not there anymore before this fix.
33 changes: 30 additions & 3 deletions apis/v1beta1/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,17 @@ func (c *AnyConfig) UnmarshalJSON(b []byte) error {
}

// MarshalJSON specifies how to convert this object into JSON.
func (c *AnyConfig) MarshalJSON() ([]byte, error) {
if c == nil {
func (c AnyConfig) MarshalJSON() ([]byte, error) {
if c.Object == nil {
return []byte("{}"), nil
}
return json.Marshal(c.Object)

nonNilMap := make(map[string]interface{}, len(c.Object))
iblancasa marked this conversation as resolved.
Show resolved Hide resolved
for k, v := range c.Object {
nonNilMap[k] = v
}

return json.Marshal(nonNilMap)
}

// Pipeline is a struct of component type to a list of component IDs.
Expand Down Expand Up @@ -280,6 +286,27 @@ func (c *Config) Yaml() (string, error) {
return buf.String(), nil
}

// MarshalJSON marshalls the Config field.
func (c Config) MarshalJSON() ([]byte, error) {
// When you marshal a struct that embeds itself or a type that directly or indirectly refers back to itself,
// Go's JSON marshaling can lead to infinite recursion. To avoid this, we create an alias of the struct
// (Config), so Go no longer considers it the same type.
// This allows you to embed the struct safely within itself without triggering that infinite loop.
iblancasa marked this conversation as resolved.
Show resolved Hide resolved
type Alias Config
// Ensure we call the custom marshaller for AnyConfig for the Receivers
receiversJSON, err := json.Marshal(c.Receivers)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we only need to do this for receivers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iblancasa bump on this

if err != nil {
return nil, err
}
return json.Marshal(&struct {
*Alias
Receivers json.RawMessage `json:"receivers,omitempty"`
}{
Alias: (*Alias)(&c),
Receivers: receiversJSON,
})
}

// Returns null objects in the config.
func (c *Config) nullObjects() []string {
var nullKeys []string
Expand Down
Loading
Loading