-
Notifications
You must be signed in to change notification settings - Fork 11
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 missing properties for backend otel configuration #52
Conversation
Signed-off-by: David Hontecillas <dhontecillas@gmail.com>
…the layers section Signed-off-by: David Hontecillas <dhontecillas@gmail.com>
I am going to review everything, and when it is ready, we will check that the documentation is also correct, and then I will convert the PR from draft to ready to review. |
@dhontecillas, in case it's useful, this config snippet works in KrakenD CE v2.7.2: {
"backend": [
{
"encoding": "no-op",
"extra_config": {
"telemetry/opentelemetry": {
"layers": {
"backend": {
"metrics": {
"static_attributes": [
{
"key": "backend_service",
"value": "clips-service"
}
]
},
"traces": {
"static_attributes": [
{
"key": "backend_service",
"value": "clips-service"
}
]
}
}
}
}
},
"method": "GET",
"url_pattern": "/clips-service-v3/health",
"host": [
"http://host.docker.internal:4042"
],
"sd": "static"
}
],
"endpoint": "/data/v3/utils/health/clips_service",
"method": "GET"
} It correctly adds the |
…cs and traces than in other places
This is wrong .. I mean, it might be working, but should not work this way.. I am going to review it carefully. Please, do not rely on this working this way. For now stick with the global config only, as it is documented for the CE version: As you can see, in the enterprise version, the format for the overrides is different (and already documented, so it should be the reference): https://www.krakend.io/docs/enterprise/telemetry/opentelemetry-by-endpoint/ Let me see how I fix this, but if we put those attributes at the endpoint and backend level, the naming should be consistent. Sorry for the inconvenience. |
@adigiorgi-clickup : we are going to address this issue here krakend/krakend-otel#36 (take into account the PR is just a draft). We want to have the feature for Community Edition, but also we must make it coherent with the Enterprise Edition. |
@alombarte I've fixed the tests, the PR is ready to review and merge. |
Fixes #51 : actually a layers property inside the backend config does not exist, what we have are some attributes organized the same way as we have it at the service level for the
layers
sections: a way to add static attributes and other params to the backend, but also to disable the stage.This is config example that allows to disable_stage for the traces part of one single backend, and we add some attributes for the other part:
See the
extra_config
parts above, and the result for a trace here with the attributes:And we can see that the call to
user/1.json
is not shown:So, it works as documented in the official documentation, but , in what is missing from the documentation is the
static_attributes
property, that allows to add more attributes.What is wrong in the official documentation, is that backend's
extra_config
->telemetry/opentelemetry
accepts theproxy
properties. We cannot disable that at this inner level.Also, in the endpoint section to override config, you can also have the
backend
section, that provides a custom config for the backends that this endpoints uses. So thebackend
part must be added to the documentation here: https://www.krakend.io/docs/enterprise/telemetry/opentelemetry-by-endpoint/#endpoint-override-of-metrics-and-traces