Skip to content

Initial Commit of Logging API#378

Merged
reyang merged 18 commits into
open-telemetry:masterfrom
open-o11y:logs-api
Oct 28, 2020
Merged

Initial Commit of Logging API#378
reyang merged 18 commits into
open-telemetry:masterfrom
open-o11y:logs-api

Conversation

@kxyr
Copy link
Copy Markdown
Contributor

@kxyr kxyr commented Oct 27, 2020

This PR is an initial commit of the Logging API for C++ #337. The goal of the Logging API is to only implement the minimum set of features that are required to capture (not send/configure) data from the user. This API closely follows the Log Data Model and Logging specification overview.

This PR adds:

  • Classes and basic methods for the Logging API, following the conventions of metrics and traces
  • Unit tests for all added classes (except LogRecord- to be finalized)

Future enhancements include:

  • Overloaded functions for each severity level (e.g. void debug(...), void trace(...), void info(...), ..., or LogDebug(...), LogTrace(...), LogInfo(...),...)
  • Macros for each severity level (e.g. LOG_ERROR, LOG_E, LOG_E_IF, ...)
  • Functions/easier ways to create a LogRecord (e.g. overloaded constructors)

The design doc will be published as part of a future PR.

Suggestions and feedback are appreciated. Further comments are also being collected on RFC #356.

-- cc @MarkSeufert @alolita @reyang @ThomsonTan @maxgolov

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 27, 2020

Codecov Report

Merging #378 into master will increase coverage by 0.05%.
The diff coverage is 98.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   94.91%   94.97%   +0.05%     
==========================================
  Files         155      162       +7     
  Lines        6882     6979      +97     
==========================================
+ Hits         6532     6628      +96     
- Misses        350      351       +1     
Impacted Files Coverage Δ
api/test/logs/logger_test.cc 97.36% <97.36%> (ø)
api/include/opentelemetry/logs/log_record.h 100.00% <100.00%> (ø)
api/include/opentelemetry/logs/logger.h 100.00% <100.00%> (ø)
api/include/opentelemetry/logs/logger_provider.h 100.00% <100.00%> (ø)
api/include/opentelemetry/logs/noop.h 100.00% <100.00%> (ø)
api/include/opentelemetry/logs/provider.h 100.00% <100.00%> (ø)
api/test/logs/logger_provider_test.cc 100.00% <100.00%> (ø)
... and 4 more

Comment thread api/include/opentelemetry/logs/log_record.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/include/opentelemetry/logs/noop.h Outdated
Comment thread api/include/opentelemetry/logs/noop.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h Outdated
void log(nostd::string_view name = "",
Severity sev = Severity::kNone,
const T &attributes = {}) noexcept
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

log() could be called without any input parameters, is this needed and useful?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch, I think we should prevent folks from calling log() without any argument.

Comment thread api/include/opentelemetry/logs/logger.h
Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/test/logs/logger_provider_test.cc Outdated
Copy link
Copy Markdown
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM, there are things not hammered out but no need to block this PR.

Comment thread api/include/opentelemetry/logs/logger.h Outdated
Comment thread api/include/opentelemetry/logs/logger.h
@kxyr kxyr marked this pull request as ready for review October 28, 2020 00:03
@kxyr kxyr requested a review from a team October 28, 2020 00:03
@reyang reyang merged commit e3bf61e into open-telemetry:master Oct 28, 2020
@kxyr kxyr changed the title Initial Commit of Logging API Add Logging API Nov 9, 2020
@kxyr kxyr changed the title Add Logging API Initial Commit of Logging API Nov 9, 2020
GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this pull request Aug 31, 2025
…eql-action-3.x

Update github/codeql-action action to v3.29.2
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.

6 participants