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

otel: hostname handling #4954

Merged
merged 4 commits into from May 16, 2024
Merged

Conversation

MrAnno
Copy link
Collaborator

@MrAnno MrAnno commented May 8, 2024

keep-hostname(yes/no) won't work in opentelemetry()/syslog-ng-otlp(), because

  • we want to allow the same port (driver) to be used for syslog-ng-otlp() and opentelemetry()
  • we always want to store the original hostname in syslog-ng-otlp()
  • we don't want to parse anything in opentelemetry(), we have a separate parser for that

Instead of implementing keep-hostname(yes/no) scenarios for these 2 sources, we ignore this flag. syslog-ng-otlp() keeps the original hostname, opentelemetry() doesn't do anything, so it falls back to using the peer address (saddr) as HOST.

(The code forces keep-hostname(yes) on both sources, but in practice the above is done due to the fallback mechanism.)

For the opentelemetry() parser, the set-hostname(yes/no) option is introduced, yes is the default.
It extracts the host.name attribute if available, or leaves the HOST field as-is (https://opentelemetry.io/docs/specs/semconv/attributes-registry/host/).

With these defaults the syslog-ng-otlp() + opentelemetry() parser scenario ends up being correct, we'll get the original hostname regardless of where the message originally came from (OTLP, syslog, etc.).

Copy link
Contributor

github-actions bot commented May 8, 2024

This Pull Request introduces config grammar changes

syslog-ng/9d3c3c3c128788a9917bace9856315dd3995e0b3 -> MrAnno/otel-source-hostname

--- a/parser
+++ b/parser

 opentelemetry(
+    set-hostname(<yesno>)
 )

@MrAnno MrAnno requested review from alltilla and bazsi May 9, 2024 14:17
const gchar *type = log_msg_get_value_with_type(msg, logmsg_handle::RAW_TYPE, &len, &log_msg_type);

/* _parse_metadata() may invalidate the returned char pointer, so a copy is made with std::string */
std::string type = log_msg_get_value_with_type(msg, logmsg_handle::RAW_TYPE, &len, &log_msg_type);
Copy link
Collaborator

Choose a reason for hiding this comment

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

[optional] we could avoid the copy/malloc by mapping the borrowed pointer to an enum and then using that to
determine how we unpack the protobuf.

The opentelemetry() source does not parse anything, so HOST can't be set
properly according to the keep-hostname() option.

Since opentelemetry() is a no-parse source, we let HOST fall back to
saddr (in LogSource).

The opentelemetry() parser should be used if one wants to keep the
original hostname.

Signed-off-by: László Várady <laszlo.varady@anno.io>
Signed-off-by: László Várady <laszlo.varady@anno.io>
This is a preparation step for adding HOST extraction.
The hostname may appear in the resource/scope metadata, or in the log
itself, so the order it is parsed matters.

Signed-off-by: László Várady <laszlo.varady@anno.io>
Signed-off-by: László Várady <laszlo.varady@anno.io>
@MrAnno MrAnno requested review from bazsi and alltilla and removed request for alltilla and bazsi May 15, 2024 08:22
@alltilla alltilla merged commit 9a374c4 into syslog-ng:master May 16, 2024
20 checks passed
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.

None yet

3 participants