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

Send fluentd logs through otel #109

Merged
merged 5 commits into from
May 4, 2021
Merged

Send fluentd logs through otel #109

merged 5 commits into from
May 4, 2021

Conversation

jrcamp
Copy link
Contributor

@jrcamp jrcamp commented Apr 22, 2021

This sends logs collected by fluentd to otelcol via fluentforward. There are
some limitations noted by TODOs that I will file issues to track but they
should not affect the common cases. It's mostly around configuring of
hec_exporter TLS settings.

It still attaches k8s metadata on the fluentd side as it uses various
annotations to construct source/sourcetype in some cases. May not be worth
trying to fix with move to filelog receiver.

Tested with agent sending directly and through gateway.

This sends logs collected by fluentd to otelcol via fluentforward. There are
some limitations noted by TODOs that I will file issues to track but they
should not affect the common cases. It's mostly around configuring of
hec_exporter TLS settings.

It still attaches k8s metadata on the fluentd side as it uses various
annotations to construct source/sourcetype in some cases. May not be worth
trying to fix with move to filelog receiver.
@jrcamp jrcamp requested a review from dmitryax April 22, 2021 01:53
# set the index field to the value found in the pod splunk.com/index annotations. if not set, use namespace annotation, or default to the default_index
index ${record.dig("kubernetes", "annotations", "splunk.com/index") ? record.dig("kubernetes", "annotations", "splunk.com/index") : record.dig("kubernetes", "namespace_annotations", "splunk.com/index") ? (record["kubernetes"]["namespace_annotations"]["splunk.com/index"]) : ("{{ .Values.logsBackend.hec.indexName | default "main"}}")}
index ${record.dig("kubernetes", "annotations", "splunk.com/index") ? record.dig("kubernetes", "annotations", "splunk.com/index") : record.dig("kubernetes", "namespace_annotations", "splunk.com/index")}
Copy link
Contributor

Choose a reason for hiding this comment

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

Look like main default value is missed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to consolidate setting of index default in the filter ** section:

com.splunk.index ${record.dig("index") ? record.dig("index") : "{{ .Values.logsBackend.hec.indexName | default "main"}}"}

I'll have to test more to be completely confident it's working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah seems to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

So this will be updated, right?

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 index isn't set or is null then it falls back to setting default value here. No need to set default everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok found it in com.splunk.index

Comment on lines +292 to +307
<filter **>
@type record_transformer
enable_ruby
<record>
com.splunk.sourcetype ${record.dig("sourcetype") ? record.dig("sourcetype") : ""}
com.splunk.source ${record.dig("source") ? record.dig("source") : ""}
com.splunk.index ${record.dig("index") ? record.dig("index") : "{{ .Values.logsBackend.hec.indexName | default "main"}}"}
</record>
remove_keys denylist,docker,kubernetes,source,sourcetype,index
</filter>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this extra filter and use com.splunk.index instead of index from the beginning? a follow up PR is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep index can, source and sourcetype can't because of the jq .record.source = ... craziness.

@jrcamp jrcamp changed the base branch from main to otel-logs April 23, 2021 20:44
@jrcamp jrcamp marked this pull request as ready for review May 4, 2021 16:52
@jrcamp jrcamp merged commit 9b37f46 into signalfx:otel-logs May 4, 2021
@jrcamp jrcamp deleted the fluentd branch May 4, 2021 16:56
@jrcamp
Copy link
Contributor Author

jrcamp commented May 4, 2021

Merging to otel-logs for followup PRs before merging to main.

@dmitryax dmitryax mentioned this pull request May 4, 2021
jrcamp added a commit that referenced this pull request May 21, 2021
Send fluentd logs through otel (#109)

This sends logs collected by fluentd to otelcol via fluentforward. There are
some limitations noted by TODOs that I will file issues to track but they
should not affect the common cases. It's mostly around configuring of
hec_exporter TLS settings.

It still attaches k8s metadata on the fluentd side as it uses various
annotations to construct source/sourcetype in some cases. May not be worth
trying to fix with move to filelog receiver.

* Fluentd cleanup (#123)
* Remove hec token
* remove ingestHost, ingestPort, ingestProtocol
* Disable receivers and pipelines if telemetry type disabled (#127)
- Fix changelog ordering
- Don't include fluentd configmap when agent not enabled
- Enable http-forwarder for all telemetry types since signalfx exporter
  sends metadata updates
* remove fluentforward from service (no longer configured in gateway)
* use <ingest>/v1/log for hec endpoint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants