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.

Sign up for GitHub

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

[SDK] support for the new OTel log API #2123

Merged
merged 20 commits into from
May 6, 2023
Merged

[SDK] support for the new OTel log API #2123

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ba48a1a
Add event name setter to log record
ThomsonTan Apr 26, 2023
35f4d3e
Support EventId in SDK logger
ThomsonTan Apr 27, 2023
08df24a
Merge branch 'main' into otel_cpp_log_sdk_impl
ThomsonTan Apr 27, 2023
9e4ef59
Merge branch 'main' into otel_cpp_log_sdk_impl
ThomsonTan Apr 27, 2023
976dc15
Add implementation for derived classes of Recordable
ThomsonTan Apr 27, 2023
8dab553
Override SetEventId for ElasticLogRecord
ThomsonTan Apr 27, 2023
4f438c4
Merge branch 'main' into otel_cpp_log_sdk_impl
ThomsonTan Apr 27, 2023
df51baa
Comment out unused parameter name
ThomsonTan Apr 28, 2023
2c97fa5
Fix more unused parameters warning
ThomsonTan Apr 28, 2023
242e364
Fix warning
ThomsonTan Apr 28, 2023
8b7785e
Avoid passing raw event Id as integer to SDK
ThomsonTan Apr 28, 2023
818cf6b
Initialize EventId in ReadWriteLogRecordable
ThomsonTan May 2, 2023
7e5d7fc
Remove unnecesssary init
ThomsonTan May 2, 2023
afac98b
Fix MD034 - Bare URL used
ThomsonTan May 2, 2023
3a40fc6
Fix CI
ThomsonTan May 3, 2023
79b6729
Update changelog
ThomsonTan May 3, 2023
cdf64f5
Implement SetEventId for multi_recordable
ThomsonTan May 3, 2023
63eff3f
Merge branch 'main' into otel_cpp_log_sdk_impl
ThomsonTan May 3, 2023
0fd54b2
Merge branch 'main' into otel_cpp_log_sdk_impl
ThomsonTan May 3, 2023
04e6dcd
Add integration test with user facing Log API call
ThomsonTan May 3, 2023
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
8 changes: 8 additions & 0 deletions api/include/opentelemetry/logs/log_record.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ class LogRecord
virtual void SetAttribute(nostd::string_view key,
const opentelemetry::common::AttributeValue &value) noexcept = 0;

/**
* Set the Event Id.
* @param id The event id to set
* @param name Optional event name to set
*/
// TODO: mark this as pure virtual once all exporters have been updated
virtual void SetEventId(int64_t id, nostd::string_view name = {}) noexcept = 0;

/**
* Set the trace id for this log.
* @param trace_id the trace id to set
Expand Down
17 changes: 15 additions & 2 deletions api/include/opentelemetry/logs/logger_type_traits.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,22 @@ template <>
struct LogRecordSetterTrait<EventId>
{
template <class ArgumentType>
inline static LogRecord *Set(LogRecord *log_record, ArgumentType && /*arg*/) noexcept
inline static LogRecord *Set(LogRecord *log_record, ArgumentType &&arg) noexcept
{
log_record->SetEventId(arg.id_, nostd::string_view{arg.name_.get()});

return log_record;
}
};

template <>
struct LogRecordSetterTrait<int64_t>
Copy link
Member

@owent owent Apr 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is it conflicted with Body?Log body can also be converted from int64_t implicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks for the catch. Seems the template part of filling logging record requires each parameter to be in different type, and the any type on body make it conflict with other parameters. Do you have any suggestion for this problem?

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea for this problem by now, and I only avoided to specialize templates for these types before.
Maybe we can add a unit test to check the mistake.

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 removed the passing of int64_t to SDK, instead, a temporary EventId struct will be constructed which should fix the issue.

For unittest, do you mean to cover all the variant types of body field?

Copy link
Member

@owent owent May 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, use EmitLogRecord to cover all the variant types of body field.
Maybe we can also add it in another PR later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI, I added an integration test to cover if the EventId passed to the user-facing API is passed correctly to the SDK.

https://github.com/ThomsonTan/opentelemetry-cpp/blob/otel_cpp_log_sdk_impl/exporters/ostream/test/ostream_log_test.cc#L465

{
template <class ArgumentType>
inline static LogRecord *Set(LogRecord *log_record, ArgumentType &&arg) noexcept
{
// TODO: set log_record
log_record->SetEventId(arg, nostd::string_view{});

return log_record;
}
};
Expand Down
10 changes: 10 additions & 0 deletions exporters/elasticsearch/include/opentelemetry/exporters/elasticsearch/es_log_recordable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ class ElasticSearchRecordable final : public sdk::logs::Recordable
*/
void SetBody(const opentelemetry::common::AttributeValue &message) noexcept override;

/**
* Set the Event Id
* @param id the event id to set
* @param name the event name to set
*/
void SetEventId(int64_t /* id */, nostd::string_view /* name */) noexcept override
{
// TODO: implement event id
}

/**
* Set the trace id for this log.
* @param trace_id the trace id to set
Expand Down
4 changes: 4 additions & 0 deletions exporters/ostream/src/log_record_exporter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ sdk::common::ExportResult OStreamLogRecordExporter::Export(
continue;
}

int64_t event_id = log_record->GetEventId();

// Convert trace, spanid, traceflags into exportable representation
constexpr int trace_id_len = 32;
constexpr int span_id__len = 16;
Expand Down Expand Up @@ -96,6 +98,8 @@ sdk::common::ExportResult OStreamLogRecordExporter::Export(
printAttributes(log_record->GetAttributes(), "\n ");

sout_ << "\n"
<< " event_id : " << event_id << "\n"
<< " event_name : " << log_record->GetEventName() << "\n"
<< " trace_id : " << std::string(trace_id, trace_id_len) << "\n"
<< " span_id : " << std::string(span_id, span_id__len) << "\n"
<< " trace_flags : " << std::string(trace_flags, trace_flags_len) << "\n"
Expand Down
10 changes: 10 additions & 0 deletions exporters/otlp/include/opentelemetry/exporters/otlp/otlp_log_recordable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ class OtlpLogRecordable final : public opentelemetry::sdk::logs::Recordable
*/
void SetBody(const opentelemetry::common::AttributeValue &message) noexcept override;

/**
* @brief Set the Event Id for this log.
* @param id the event Id to set
* @param name the event name to set
*/
void SetEventId(int64_t id, nostd::string_view name) noexcept override
{
// TODO: export Event Id to OTLP
}

/**
* Set the trace id for this log.
* @param trace_id the trace id to set
Expand Down
8 changes: 8 additions & 0 deletions sdk/include/opentelemetry/sdk/logs/multi_recordable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ class MultiRecordable final : public Recordable
*/
void SetBody(const opentelemetry::common::AttributeValue &message) noexcept override;

/**
* Set the Event Id
* @param id the event id to set
* @param name the event name to set
*/
// TODO: implement set event id
void SetEventId(int64_t /* id */, nostd::string_view /* name */) noexcept override {}
owent marked this conversation as resolved.
Show resolved Hide resolved

/**
* Set the trace id for this log.
* @param trace_id the trace id to set
Expand Down
22 changes: 22 additions & 0 deletions sdk/include/opentelemetry/sdk/logs/read_write_log_record.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,25 @@ class ReadWriteLogRecord final : public ReadableLogRecord
*/
const opentelemetry::common::AttributeValue &GetBody() const noexcept override;

/**
* Set the Event Id object
* @param id the event Id to set
* @param name the event name to set
*/
void SetEventId(int64_t id, nostd::string_view name) noexcept override;

/**
* Get event Id of this log.
* @return the event Id of this log.
*/
int64_t GetEventId() const noexcept override;

/**
* Get event name of this log.
* @return the event name of this log.
*/
nostd::string_view GetEventName() const noexcept override;

/**
* Set the trace id for this log.
* @param trace_id the trace id to set
Expand Down Expand Up @@ -176,6 +195,9 @@ class ReadWriteLogRecord final : public ReadableLogRecord
opentelemetry::common::SystemTimestamp timestamp_;
opentelemetry::common::SystemTimestamp observed_timestamp_;

int64_t event_id_;
std::string event_name_;

// We do not pay for trace state when not necessary
struct TraceState
{
Expand Down
12 changes: 12 additions & 0 deletions sdk/include/opentelemetry/sdk/logs/readable_log_record.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@ class ReadableLogRecord : public Recordable
*/
virtual const opentelemetry::common::AttributeValue &GetBody() const noexcept = 0;

/**
* Get the Event id.
* @return the event id
*/
virtual int64_t GetEventId() const noexcept = 0;

/**
* Get the Event Name.
* @return the event name
*/
virtual nostd::string_view GetEventName() const noexcept = 0;

/**
* Get the trace id of this log.
* @return the trace id of this log
Expand Down
16 changes: 16 additions & 0 deletions sdk/src/logs/read_write_log_record.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,22 @@ const opentelemetry::common::AttributeValue &ReadWriteLogRecord::GetBody() const
return body_;
}

void ReadWriteLogRecord::SetEventId(int64_t id, nostd::string_view name) noexcept
{
event_id_ = id;
event_name_ = std::string{name};
}

int64_t ReadWriteLogRecord::GetEventId() const noexcept
{
return event_id_;
}

nostd::string_view ReadWriteLogRecord::GetEventName() const noexcept
{
return nostd::string_view{event_name_};
}

void ReadWriteLogRecord::SetTraceId(const opentelemetry::trace::TraceId &trace_id) noexcept
{
if (!trace_state_)
Expand Down
2 changes: 2 additions & 0 deletions sdk/test/logs/batch_log_record_processor_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class MockLogRecordable final : public opentelemetry::sdk::logs::Recordable

void SetBody(const std::string &message) noexcept { body_ = message; }

void SetEventId(int64_t id, nostd::string_view name) noexcept override {}

void SetTraceId(const opentelemetry::trace::TraceId &) noexcept override {}

void SetSpanId(const opentelemetry::trace::SpanId &) noexcept override {}
Expand Down
2 changes: 2 additions & 0 deletions sdk/test/logs/logger_provider_sdk_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ class DummyLogRecordable final : public opentelemetry::sdk::logs::Recordable

void SetBody(const opentelemetry::common::AttributeValue &) noexcept override {}

void SetEventId(int64_t id, nostd::string_view event_name) noexcept override {}

void SetTraceId(const opentelemetry::trace::TraceId &) noexcept override {}

void SetSpanId(const opentelemetry::trace::SpanId &) noexcept override {}
Expand Down
8 changes: 8 additions & 0 deletions sdk/test/logs/logger_sdk_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class MockLogRecordable final : public opentelemetry::sdk::logs::Recordable

void SetBody(const std::string &message) noexcept { body_ = message; }

void SetEventId(int64_t id, nostd::string_view name) noexcept override
{
event_id_ = id;
log_record_event_name_ = static_cast<std::string>(name);
}

void SetTraceId(const opentelemetry::trace::TraceId &trace_id) noexcept override
{
trace_id_ = trace_id;
Expand Down Expand Up @@ -144,6 +150,8 @@ class MockLogRecordable final : public opentelemetry::sdk::logs::Recordable
private:
opentelemetry::logs::Severity severity_ = opentelemetry::logs::Severity::kInvalid;
std::string body_;
int64_t event_id_;
std::string log_record_event_name_;
opentelemetry::trace::TraceId trace_id_;
opentelemetry::trace::SpanId span_id_;
opentelemetry::trace::TraceFlags trace_flags_;
Expand Down
2 changes: 2 additions & 0 deletions sdk/test/logs/simple_log_record_processor_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class TestLogRecordable final : public opentelemetry::sdk::logs::Recordable

void SetBody(const char *message) noexcept { body_ = message; }

void SetEventId(int64_t id, nostd::string_view name) noexcept override {}

void SetTraceId(const opentelemetry::trace::TraceId &) noexcept override {}

void SetSpanId(const opentelemetry::trace::SpanId &) noexcept override {}
Expand Down