-
Notifications
You must be signed in to change notification settings - Fork 438
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
Otlp http log example #1062
Otlp http log example #1062
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1062 +/- ##
=======================================
Coverage 94.41% 94.41%
=======================================
Files 158 158
Lines 6077 6077
=======================================
Hits 5737 5737
Misses 340 340 |
/easycla |
…_OTLP_GRPC is enabled. (open-telemetry#1061)
…telemetry-cpp into otlp-http-log-example
namespace logs_api = opentelemetry::logs; | ||
|
||
namespace sdktrace = opentelemetry::sdk::trace; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - use namespace alias as trace_sdk
, logs
and logs_sdk
for consistency with other places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
defines = ["BAZEL_BUILD"], | ||
deps = [ | ||
"//api", | ||
"//sdk:headers", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need sdk::headers, I see this included in earlier PRs too so probably I am missing something :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's needed for "opentelemetry/sdk/version/version.h"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks for letting me know. Not related to this PR, but we may have to revisit this afterward as ideally instrumentation libraries should have no dependency on SDK.
nostd::shared_ptr<logs::Logger> get_logger() | ||
{ | ||
auto provider = logs::Provider::GetLoggerProvider(); | ||
return provider->GetLogger("foo_library_logger", nostd::span<nostd::string_view>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return provider->GetLogger("foo_library_logger");
will be much simpler ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
logger->Log(opentelemetry::logs::Severity::kDebug, "name", "body", | ||
std::map<std::string, std::string>(), std::map<std::string, std::string>(), | ||
ctx.trace_id(), ctx.span_id(), ctx.trace_flags(), | ||
opentelemetry::common::SystemTimestamp()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested it, but we should be able to simplify this by using the initializer-list for resources and attributes ( refer API tests for logger for example):
logger->Log(opentelemetry::logs::Severity::kDebug, "name", "body", {}, {}, ctx.trace_id(), ctx.span_id(), ctx.trace_flags(), opentelemetry::common::SystemTimestamp());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
otlp http log example
Changes
Adds otlp http log example
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes