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

Add messages to support Log data type #151

Merged

Conversation

tigrannajaryan
Copy link
Member

@tigrannajaryan tigrannajaryan commented May 14, 2020

This definition is based on the accepted log data model:
https://github.com/open-telemetry/oteps/blob/master/text/logs/0097-log-data-model.md

LogsService is defined similarly to TraceService and MetricsService.

@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-approvers please review.

Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

in general LGTM as it matches the proposed otep. As long as we know it's in alpha and changes are possible, should be ok.

Also, it will be good to materialize OTEP in a specs repo

A few minor comments about naming... so request changes

opentelemetry/proto/common/v1/common.proto Outdated Show resolved Hide resolved
opentelemetry/proto/common/v1/common.proto Outdated Show resolved Hide resolved
opentelemetry/proto/logs/v1/logs.proto Show resolved Hide resolved
opentelemetry/proto/logs/v1/logs.proto Outdated Show resolved Hide resolved
opentelemetry/proto/logs/v1/logs.proto Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member Author

@SergeyKanzhelev thanks for reviewing. I am experimenting with a slightly different approach. Let me mark this PR as WIP and I will update it once I finish the experiments.

@tigrannajaryan tigrannajaryan changed the title Add messages to support Log data type [WIP] Add messages to support Log data type Jun 3, 2020
@tigrannajaryan
Copy link
Member Author

I will return to this once #157 is merged.

@tigrannajaryan tigrannajaryan changed the title [WIP] Add messages to support Log data type Add messages to support Log data type Jun 16, 2020
@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-approvers please have a look. I updated the PR.

This definition is based on the accepted log data model:
https://github.com/open-telemetry/oteps/blob/master/text/logs/0097-log-data-model.md

LogsService is defined similarly to TraceService and MetricsService.
Copy link

@zenmoto zenmoto left a comment

Choose a reason for hiding this comment

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

This looks good to me.

opentelemetry/proto/logs/v1/logs.proto Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member Author

@SergeyKanzhelev can you please review.

opentelemetry/proto/logs/v1/logs.proto Outdated Show resolved Hide resolved
opentelemetry/proto/logs/v1/logs.proto Outdated Show resolved Hide resolved
opentelemetry/proto/logs/v1/logs.proto Outdated Show resolved Hide resolved
opentelemetry/proto/logs/v1/logs.proto Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member Author

@SergeyKanzhelev can you please have another look. You requested changes, I believe everything is addressed now.

@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-approvers please review.

@bogdandrutu bogdandrutu requested a review from a team as a code owner July 15, 2020 18:48
@bogdandrutu bogdandrutu merged commit 1d26e52 into open-telemetry:master Jul 15, 2020
@tigrannajaryan tigrannajaryan deleted the feature/tigran/logproto branch July 15, 2020 19:28
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

8 participants