First cut at Entity API + SDK specification. #4565
Conversation
Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>
dyladan
left a comment
dyladan
left a comment
There was a problem hiding this comment.
Overall there's nothing surprising here, which is a good thing. I think most of the prototypes are pretty close to this anyway since the merge algorithm is the only real technical part.
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
|
Working on the JS prototype today. We don't have the EntityProvider but everything else is basically there. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
jsuereth
changed the title
dyladan
left a comment
dyladan
left a comment
There was a problem hiding this comment.
I like this version a lot better
| #### Get the active Resource | ||
|
|
||
| This API MUST return the current `Resource` for which Telemetry | ||
| is being reported. |
There was a problem hiding this comment.
Does this always return the same resource? If so, what is the reason to have a ResourceProvider? We may want to consider the case where there are multiple resources in an application (different per-signal or per-TracerProvider, etc).
There was a problem hiding this comment.
For now, yes.
I see a parallel in having multiple resources. Today if you want multiple resources, you have multiple {Signal}Provider. You'd do the same for ResourceProvider, and you register these with each {Signal}Provider} so It's a pairing.
In Java prototype, I accomplished this via the OpenTelemetry API -> see https://github.com/open-telemetry/opentelemetry-java/pull/7434/files#diff-fae9ff5f52df2c944624272d9bd01cf361c8bea7a172f6536f74c260231d47ffR76
| ### Attach an Entity | ||
|
|
||
| The effect of calling this API is to attach (or update) an Entity | ||
| to the current `Resource`. |
There was a problem hiding this comment.
I think we should be a lot more specific about the update behavior. Is it a full replacement or a merge? If it is a merge, is there a way to remove descriptive attributes? What is done with invalid entries (e.g. different identity but same entity type, which iirc is not allowed).
There was a problem hiding this comment.
I was specific in the SDK but not the API.
Agree we need to be explicit, I can move that detail to API, but in other areas of the spec, those details are left to the SDK specification.
In practice, I think an implementor of the API would want those details in the API definition, so I'm fine having it defined in the API specification on expected behavior.
There was a problem hiding this comment.
I guess it could be left to the SDK. It depends if we want to allow third party SDKs to have different behavior. The actual logic is implemented in the SDK so I think that's probably the right place for it. I talked myself out of my own comment. I'd leave it in SDK spec.
| This SDK MUST implement the | ||
| [Get the active Resource API](api.md#get-the-active-resource). | ||
|
|
||
| The SDK MUST provide exactly one `Resource` per `ResourceProvider` via this API. |
There was a problem hiding this comment.
If a ResourceProvider can only ever provide a single resource, why have a provider at all?
There was a problem hiding this comment.
-- my updated thinking below --
There was a problem hiding this comment.
I need to formulate my thoughts more for a response. High level -
We want entities not attached to the "active" resource. I.e. we may have another mechanism to report other resource (which should not be associated with telemetry from this SDK), but may report Entity events and relationships.
So this limitation of only one resource applies to the "active" resource for which the SDK is reporting telemetry against.
|
@jsuereth glad to see this landing! A couple of things: I believe that the Java SIG already has something unrelated to this called a But also, it would be helpful to use the term One other point: The EntityProvider OTEP still hasn't been merged. It keeps getting closed automatically because there is no further work to do on it. If we are satisfied with that design, please merge the OTEP before merging the spec changes defined in that OTEP. 🙏 |
|
@tedsuo Regarding The issue of discovery and confusion is real and present in my prototype, so that's worth evaluating. This PR is still DRAFT. It's an update of OTEP 256 (merged/approved) but as we made specification we became increasingly certain this needs to be an API. What we need for OTEP 256 is somewhere between your OTEP and the original, i.e. your OTEP expands scope a bit more than we need, but also we should address concerns. As discussed in the specification meeting today, I think this is the path forward:
I think each of these points should be discussed and sorted out. I'm planning to write up issues to discuss / design them, let me know if there are other major concerns you think we need to address here. |
| The `Resource` is responsible for emitting `Entity`s and tracking | ||
| the current set of entities attached to the `Resource`. | ||
|
|
||
| The `Resource` MUST provide a function to: |
There was a problem hiding this comment.
Is this resource equivalent to the SDK's current representation of a resource? I.e. can the API caller access the resource to get its attributes, even if it doesn't intend to attach an entity?
This idea of whether the resource should be in any way accessible to instrumentation or distributions via getters in the API or SDK has come up a number of times in opentelemetry-java. I've resisted adding such accessors because 1. couldn't get a solid use case that couldn't be solved more idiomatically through different means 2. the spec doesn't mention these types of accessors.
There was a problem hiding this comment.
Is this resource equivalent to the SDK's current representation of a resource? I.e. can the API caller access the resource to get its attributes, even if it doesn't intend to attach an entity?
This is an option for the SDK. Due to binary compatibility reasons, the approach I took in the Java prototype was to have an API Resource and an SDK Resource. The SDK Reource is read-only and allows other aspects of the SDK to use / write the resource in export paths. The API Resource is write-only, and allows users to add (or update) entities on the Resource.
Regarding solid use cases for read in the API path, I do not see a need either and this draft does not propose that.
There was a problem hiding this comment.
👍
I do think the reuse of a Resource type in different contexts (API and SDK) will be a point of contention for maintainers, and a point of confusion for users.
I get that the goal is symmetry. Just as TracerProvider provides Tracers, MeterProvider provides Meters, LoggerProvider provides Loggers, it follows that a ResourceProvider would provide a Resource.
And here, the Resource provided by ResourceProvider is an interface that instrumentations use to update entities.
But Resource is now overloaded. Its now both a sort of record / pojo / struct style dumb data carrier, and the entity analog of a Meter / Logger / Tracer.
I'm trying to brainstorm for alternative words that describe this concept.
- If a meter is the entry point for instruments
- And a tracer is the entry point for spans
- And a logger is the entry point for logs
- Then, what is the name of the entry point for entities?
Maybe something like "Resourcer"? The "er" suffix seems weird, but follows the pattern meter / tracer / logger, and avoid collision with the "Resource" SDK concept.
There was a problem hiding this comment.
We can use EntityProvider - I agree name will be the most contentious part, I just haven't found a good one yet.
There was a problem hiding this comment.
Well naming aside, I think these things make sense conceptually
| If the incoming `Entity`'s `entity_type` property matches an existing `Entity` | ||
| in the current set AND the `identity` attributes for these `Entity`s are | ||
| different, then the SDK MUST ignore the new entity. |
There was a problem hiding this comment.
How would we handle the hw namespace where we would either have 1 hw entity definition or possibly 1 per hw.type if we could have fixed attributes? It gets trickier as we could have multiple of the 1 hw type ie 4 disks in a raid array.
There was a problem hiding this comment.
Two things:
- This API, for now, only accounts for attaching entities to active resource. So if you're reporting data against each entity individually, you'd have to create different SDKs for each entity (as you would today, to get different resources).
- The Entity SIgnal (Phase 2 - where we add relationships) will have a new API where you can declare relationships about entities without attaching to the active Resource. The details of that are still TBD.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
github-project-automation
bot
moved this from Phase 1B: EntityManager OTEP
to Done
in Entities: Phase 1
7 participants
Adds the initial Entity SDK specification, from OTEP#264
Changes
Adds an API with the following components:
ResourceProviderResourceThe API allows the attaching of
EntitytoResource, in addition to updating descriptive attributes for previously reportedEntitys.Adds an SDK with the following components:
ResourceProviderResourceDoes NOT address the following:
Resourcemechanisms of SDK andEntityProviderPrototypes