-
Notifications
You must be signed in to change notification settings - Fork 893
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 middleware to process semantic conventions #1194
Conversation
As discovered while analyzing the request from #1143, it became apparent that the current semantic conventions do lack the information about the middleware running on the runtime within the process. Adding this information makes the traces more valuable - the proposed change makes it possible to visualize that Tomcat 9.0.39 application server is sending messages to a Kafka message queue which in turn has a subscriber running on GlassFish 5.0 persisting the content to Druid 0.15.0. In the current form, it seems that the semantic conventions leave this information at the span level, which would be creating redundancy in outbound communication from the agent and/or not capture the information in unified manner, making it harder for Otel-compliant servers to visualize such traces.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not add this to process. All resources are process-wide, the middleware is IMHO not really more closely related to the process than, say the faas.instance_id.
CC @tigrannajaryan (cf #882, #1137)
|
||
| Attribute | Description | Example | Required | | ||
|---|---|---|--| | ||
| process.middleware.type | Standardized attribute, specifying the type of middleware on the runtime. This attribute adds value to traces, giving the opportunity to visualise different kind of middleware using different representation in server-side. The supported values of type are `as` (application server), `ws` (web server), `dbs` (database server) or `mom` (message-oriented middleware). | `as` | No | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a database server middleware?
The means of communicating with a database server, such as JDBC, is middleware, but I'm not sure that the database server itself is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being new to Otel I might be on a wrong track here as the databases seem to be treated as downstream dependencies vs part of the trace itself. But there is no reason why there could not be agents monitoring the databases themselves and building spans out of the database as well. And for example with Druid and other JVM-based databases, it is rather straightforward as of now. Also the middleware definition lists databases as one of the members of middleware family.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which "middleware definition" are you referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I was too optimistic when refering to it as "definition", as for example Wikipedia version of it does not count the database as middleware and maybe rightfully so. This however does not change the fact that many of the databases (Druid, Cassandra, Derby) do run on a Java runtime and could very well be part of the trace itself (vs downstream dependencies). That being said, maybe Otel does treat databases as not part of the trace at all or treats them special members within the trace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think midleware is not really what you mean anyway. It is used by many webframeworks to mean something different, e.g. https://docs.djangoproject.com/en/3.1/topics/http/middleware/ or https://aspnetmonsters.com/2016/03/2016-02-28-what-is-middleware-anyway/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are numerous traces in the wild internet, trying to define middleware. And maybe I am off the best path here with the naming, but it does not seem to change the need to enhance the spans in the trace with the information about what kind of "middleware" they are running. This is also something that most existing (non-otel-compliant) vendors offer and having this clearly defined in Otel should greatly reduce the friction in migrating from an existing APM vendor to standards-based solutions.
|
Changed the description, removed the hierarchy of process-runtime-middleware, as middleware can be directly running on process as well.
Middleware example now more clearly highlights the value of different attributes. Also rephrased the MUST and SHOULD categories for middleware type and name.
That is not the point. The resource namespaces should make the name unique, not describe some hierarchy. Also, many |
@ivomagi please sign the CLA. |
I signed it |
This is correct and has been our approach so far. Namespaces are not intended to describe a hierarchy of objects. Namespaces are intended to ensure global uniqueness of names. There is no need to create nested namespaces just because objects also logically are nested, although it is of course not prohibited. |
This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry#789 (comment) open-telemetry#882 (comment) open-telemetry#1194 (comment)
Conceptually these seem to live at the layer of InstrumentationLibrary - they aren't really constant to the app (one app can have multiple middlewares) but can still group some spans together. Does it make sense to add information about the instrumented library in that struct? |
I had the impression this PR is about a different kind of "middleware" that is more or less an aditional runtime within the language runtime, such as an Java AppServer (you can have only one per JVM -- at least usually) |
I signed it |
Apparently this is the path Steve Flanders also recommended to take in the conversation I was having on the exact time you provided this alternative. Will be closing the PR and will use the InstrumentationLibrary route to send the information. |
Will be using the InstrumentationLibrary.name and InstrumentationLibrary.version attributes which would (in case the OTLP protocol is used) be converted in the collector to otel.library.name and otel.library.version and sent to server. |
While this is true, the values are not suitable for use as InstrumentationLibrary. A new concept on the same layer would be needed, e.g. open-telemetry/oteps#78 |
Well, it seems the InstrumentationLibrary.name and InstrumentationLibrary.version is not at the moment used to send this information. It seems that at least Java and PHP agents use these properties to send the module instrumenting the span and the module version. Two examples:
|
This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: #789 (comment) #882 (comment) #1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
…etry#1197) This topic has come up at least 3 times now. I believe a clarification is warranted. The clarification is aligned with previous recommendations: open-telemetry/opentelemetry-specification#789 (comment) open-telemetry/opentelemetry-specification#882 (comment) open-telemetry/opentelemetry-specification#1194 (comment)
As discovered while analyzing the request from #1143, it became apparent that the current semantic conventions do lack the information about the middleware running on the runtime within the process. Adding this information makes the traces more valuable - the proposed change makes it possible to visualize that Tomcat 9.0.39 application server is sending messages to a Kafka message queue which in turn has a subscriber running on GlassFish 5.0 persisting the content to Druid 0.15.0. In the current form, it seems that the semantic conventions leave this information at the span level, which would be creating redundancy in outbound communication from the agent and/or not capture the information in unified manner, making it harder for Otel-compliant servers to visualize such traces.
Fixes #1143
Changes
Added middleware definition to the resource semantic conventions.
Related issues #
Related oteps #