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

Declare logs Stable #376

Merged

Conversation

tigrannajaryan
Copy link
Member

The log data model is now declared Stable: open-telemetry/opentelemetry-specification#2387

The OTLP proto in this repository now matches the log data model
in the specification.

This PR declares OTLP proto for logs Stable as well.

--

Please review the messages related to logs in this repository and make sure they match the log data model in the spec.
If you have any other concerns with regards to declaring the logs Stable please voice them.

The log data model is now declared Stable: open-telemetry/opentelemetry-specification#2387

The OTLP proto in this repository now matches the log data model
in the specification.

This PR declares OTLP proto for logs Stable as well.
@codeboten
Copy link
Contributor

codeboten commented Mar 31, 2022

Will moving forward the stability of JSON support be possible with logs being declared stable? #230

@tigrannajaryan
Copy link
Member Author

Will moving forward the stability of JSON support be possible with logs being declared stable? #230

JSON for all signal types (not just logs) is explicitly declared Alpha for now. Declaring OTLP Logs stable will not change that. We will work on stabilizing JSON separately.

@jmacd jmacd mentioned this pull request Apr 1, 2022
@tigrannajaryan
Copy link
Member Author

Here is the audit that shows all log data model fields are present in OTLP:

Data Model OTLP
Timestamp LogRecord.time_unix_nano
ObservedTimestamp LogRecord. observed_time_unix_nano
TraceId LogRecord.trace_id
SpanId LogRecord.span_id
TraceFlags LogRecord.flags & TRACE_FLAGS_MASK
SeverityText LogRecord.severity_text
SeverityNumber LogRecord.severity_number
Body LogRecord.body
Resource ResourceLogs.resource
InstrumentationScope ScopeLogs.scope
Attributes LogRecord.attributes

@tigrannajaryan
Copy link
Member Author

I would like to wait for open-telemetry/opentelemetry-log-collection#466 to be merged and then consumed as a dependency in the Collector before we declare logs stable (that log-collection library PR makes changes to Timestamp/ObservedTimestamp column as defined by logs data model). We do not expect any pushback or log data model stability as a result of log-collection library changes, but I still would like to move slowly to give enough time for any unknown issues to be discovered.

@tigrannajaryan
Copy link
Member Author

@djaglowski I see that open-telemetry/opentelemetry-log-collection#466 is now merged. I want to wait until the changes it introduces are also used in the Collector and we confirm that everything works nicely. Can you ping me after that is done so that we can move forward this stability declaration here?

@djaglowski
Copy link
Member

@tigrannajaryan, yes, I know a a couple minor changes that need to happen. I'll also do a final audit to make sure we haven't missed anything. Will let you know.

@djaglowski
Copy link
Member

The final issue that I am aware of has been addressed as of this morning.

@tigrannajaryan
Copy link
Member Author

I want to also finish the removal of LogRecord Name field from the Collector codebase. It is currently blocked on open-telemetry/opentelemetry-collector-contrib#9258

@tigrannajaryan
Copy link
Member Author

tigrannajaryan commented May 2, 2022

open-telemetry/opentelemetry-collector-contrib#9258 is now resolved. All recent changes to the logs proto are now propagated to the Collector and have been used without any problems.

There are no known issues that block log proto stability declaration that I am aware of. I believe this is now ready to move forward.

@tigrannajaryan tigrannajaryan changed the title [DO NOT MERGE] Declare logs Stable Declare logs Stable May 2, 2022
@tigrannajaryan tigrannajaryan marked this pull request as ready for review May 2, 2022 15:11
@tigrannajaryan tigrannajaryan requested a review from a team May 2, 2022 15:11
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

There are known issues that block log proto stability declaration that I am aware of. I believe this is now ready to move forward.

@tigrannajaryan did you mean there are no known issues?

@tigrannajaryan
Copy link
Member Author

There are known issues that block log proto stability declaration that I am aware of. I believe this is now ready to move forward.

@tigrannajaryan did you mean there are no known issues?

Yes, updated, it was a typo.

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

we should get couple more +1 from log SIG members

@tigrannajaryan
Copy link
Member Author

Let's merge this after 0.17.0 is released.

@tigrannajaryan
Copy link
Member Author

This looks good to go. Merging.

@tigrannajaryan tigrannajaryan merged commit d6481ea into open-telemetry:main May 10, 2022
@tigrannajaryan tigrannajaryan deleted the declare-logs-stable branch May 10, 2022 22:47
tigrannajaryan added a commit to open-telemetry/opentelemetry-specification that referenced this pull request May 20, 2022
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this pull request May 20, 2022
tigrannajaryan added a commit to tigrannajaryan/opentelemetry-specification that referenced this pull request May 20, 2022
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.