Rename Logs Bridge API to Logs API and do not discourage from direct usage #4235
Conversation
Co-authored-by: Cijo Thomas <[email protected]>
| @@ -81,7 +81,7 @@ Emit a `LogRecord` representing an `Event`. | |||
| **Implementation Requirements:** | |||
|
|
|||
| The implementation MUST use the parameters | |||
| to [emit a logRecord](./bridge-api.md#emit-a-logrecord) as follows: | |||
| to [emit a logRecord](./api.md#emit-a-logrecord) as follows: | |||
There was a problem hiding this comment.
This is still part of the open questions around the Logs API, and I have the same set of concerns around the bait and switch that this could lead to.
IMHO: We SHOULD have a emitEvent as part of the public API and which this is the Event-SDK (that we have general agreement on that this is going away), this has other context implications that are being "bypassed" by not addressing the Open questions around the "public" Logs API FIRST!
There was a problem hiding this comment.
we have to find a way to make incremental progress
let's see if we can get agreement here that the Logs Bridge API is the starting point for the Logs API
after that we can tackle:
| @@ -55,9 +55,7 @@ both their `Attributes` and their `Body`. | |||
| ## Event API use cases | |||
|
|
|||
| The Events API was designed to allow shared libraries to emit high quality | |||
| logs without needing to depend on a third party logger. Unlike the | |||
| [Logs Bridge API](./bridge-api.md), instrumentation authors and application | |||
There was a problem hiding this comment.
While I'm happy to see the kluge get deleted (because it was the ONLY Logs API that existed), it really should be that we are "merging" this functionality into the new "Logs API" and not just "bypassing" the harder questions.
| @@ -1,7 +1,10 @@ | |||
| # Logs Bridge API | |||
| # Logs API | |||
|
|
|||
| **Status**: [Stable](../document-status.md), except where otherwise specified | |||
There was a problem hiding this comment.
And this is really part of the problem, as we (general OpenTelemetry community) have agreed that
We have not agreed that the existing Stable Bridge API is that API.
There was a problem hiding this comment.
We have not agreed that the existing Stable Bridge API is that API.
this PR is explicitly asking for that agreement
fwiw, this was already in the spec:
There was a problem hiding this comment.
Then I current reject this option, and have #4236 and OTep # 267 as the reasons why -- because we don't have agreement.
There was a problem hiding this comment.
As I believe the Events API would be a more natural starting point because we are introducing the ability to emit Log Records as this new things we called Event in the OTep, But we didn't agree to anything in the OTep beyond that there is Something called an Event (which is a type of Log) and there will be a user-facing Logs API that will provide the ability to Emit these things called Events.
There was a problem hiding this comment.
@MSNev , the log is declared stable. There's no going back from that. There's also no technical requirement that the current API fails to solve as a starting point, so continuing from our current stable starting point does not actually create a problem.
I completely understand that the approach we are taking is not your favorite approach. But we have to make personal compromises in order to move the project forward. Not everything we do in the spec matches the exact way I would like it to be. But if it matches the technical requirements, has buy in from the community, and is backwards compatible, then I will work to polish that approach. And after it's all done I end up liking it anyways, because we did the hard work to cleanly meet the requirements and at the end of the day, that's what matters.
All I'm asking is that you do the same for us. After incredibly extensive discussion there are 21 approvals for open-telemetry/oteps#265, that is community agreement to continue with the approach we are taking. You can't block the entire project as a lone vote when you haven't convinced anyone that there is actually a problem!
Honestly, I'm not even sure what you are objecting to! It sounds like you are incredibly against the chosen course of action, but when I see your PRs, they appear to just be continuing on that course! It feels like there is a huge amount of philosophizing mixed in with your technical proposals, and it's really making it difficult for me to understand what you actually want. Is it just a reframing of what we are doing? It sounds like on the one hand you're trying to blow everything up and on the other hand stay the course. I really want to make this work but I don't know what to do.
These are not the baby steps, I'm looking for. This is a bit of a side-step and has broader implications. ie. This is Baby falling down the stairs past all of the hairy monsters to end up in the wrong place. |
|
It feels odd to remove the "not user facing" bits of the stable document before adding the portions that users are intended to call. I understand that the goal is to make incremental progress, but releasing in this state produces a confusing read for the uninformed reader. Should we commit to completing the followup steps before the next release? Can we add a "not user facing" qualification to the "emit log record" operation? |
Why? This is one of the goals to make it user facing. |
I thought the goal was to extend the log API with an "emit event" operation with the types of ergonomics expected out of a user facing API. The lower level (and bad ergonomics) "emit log record" operation should still be targeted at log appenders. |
And this is WHY I created my version of this PR with an EMPTY (to be worked on) section of the user API and not inherit and re-use any existing API that has not been agreed or started to be defined. |
|
@trask To summarize and expand what I said in the spec meeting:
|
|
Closing in favor of #4236 where we are implementing @tigrannajaryan's #4235 (comment) |
Part of open-telemetry/oteps#265
(extra minimal extraction from @pellared's #4225)