[processor/resourcedetection] Fix heroku config interface #24355
Conversation
github-actions
bot
added
the
processor/resourcedetection
dmitryax
force-pushed
the
fix-heroku-attribute
branch
from
67e8b2f
to
8dfeed0
Compare
dmitryax
changed the title
Technically yes, but it's more like a bug fix of a wrong configuration option introduced in the latest version. Not sure if we have strict guidelines for such case |
| @@ -5,8 +5,6 @@ all_set: | |||
| enabled: true | |||
| heroku.app.id: | |||
| enabled: true | |||
| heroku.app.name: | |||
There was a problem hiding this comment.
I know this was broken behavior, but would it make sense to transition out this option since removing it is a breaking change?
There was a problem hiding this comment.
Like keeping it as no-op with a warning for a couple of versions? It'll be just a few manual changes to support that. Resource attributes don't yet have this if_enabled_set stuff. I was actually working towards adding that when I found this issue. I don't believe anyone applied this since it was introduced only last version. If you and @TylerHelmuth think that it's worth the effort, I can apply the suggestion.
There was a problem hiding this comment.
If it was only applied in the last release, you're right that it's unlikely to be used, so I'm okay breaking it. The closest thing I can find that would document what we should do here is the stability guarantees for scraping receivers, which only says we SHOULD follow the transition process for beta components.
|
I appreciate the fix, happy to follow up if we release it in stages |
Fix config option for `service.name` attribute set by heroku detector. It was added under mismatching `heroku.app.name` config option
dmitryax
force-pushed
the
fix-heroku-attribute
branch
from
2d98abb
to
396491a
Compare
Fix Heroku config option for the
service.nameandservice.versionattributes.service.nameandservice.versionattributes were mistakenly controlled byheroku.app.nameandheroku.release.versionoptions underresource_attributesconfiguration introduced in #23253. This PR fixes the issue by using the correct config options named the same as the attributes.