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

Add ability to configure mappings from fluent attributes to otel fields #5173

Closed
wants to merge 2 commits into from

Conversation

gillg
Copy link
Contributor

@gillg gillg commented Sep 9, 2021

Description:
Help to solve : #14718

This change allow to create a very basic mapping as kind of whitelist
You can "move" an attribute data to severyty text if the key is found, or "move" attributes to structured body data (attributemap).

Then, if you prefer a string body you can convert it.

Testing:
Nothing for now

Documentation:
Some notes about the new mappings

return errors.New("`endpoint` not specified")
}

/*if c.BodyAsString == nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How to test if the config is defined or not to have a default value ?

}*/

if len(c.Mappings.Body) == 0 {
c.Mappings.Body = []string{"log", "message"}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default value for retrocompatibility.
Both log or message will be considered as "body" value

Here is a basic example config that makes the receiver listen on all interfaces
on port 8006:

```yaml
receivers:
fluentforward:
endpoint: 0.0.0.0:8006
body_as_string: true
mappings:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If anyone has a better idea / strategy about these mapping I take it. It makes the job but I have the feeling that it's not ideal.
But what I think is the concept could be reused on other receivers where underlying technology doesn't fit with all OTEL concepts.

attrs := lr.Attributes()
bodyMap := lr.Body().MapVal()
lr.SetName("fluent[d|bit]")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arbitrary name to provide something in name (I think otel collector should at least show the good exemple)

default:
return fmt.Errorf("cannot convert message type %T to string", val)
}
s, err := ValueToString(val)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To move into if Contains(conf.Mappings.Severity, key)

}

if Contains(conf.Mappings.Body, key) {
insertToAttributeMap(key, val, &bodyMap)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By simplicity, I try to do the best to convert a maximum of fields in the body, se I work on body attributemap instead of string value.

} else {
insertToAttributeMap(key, val, &attrs)
}
}
if conf.BodyAsString {
lr.Body().SetStringVal(lr.Body().AsString())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the user prefers a string body, we convert it here.

This will change a little the actual behavior, for a basic use case where you convert the log field to body we will have something like {"log": "raw string"} (really not sure about the output format of lr.Body().AsString()) instead of raw string

Any concern about that or the encoding choice ?

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 8, 2021
@gillg
Copy link
Contributor Author

gillg commented Oct 8, 2021

Anyone to review or help me to fix tests ?
Any opinions on my comments ?

@github-actions github-actions bot removed the Stale label Oct 9, 2021
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 30, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2021

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants