[chore] [mdatagen] Mdatagen hack metadata #21464
Conversation
github-actions
bot
added
cmd/mdatagen
github-actions
bot
requested review from
antonblock,
atoulme,
BinaryFissionGames,
codeboten,
crobert-1,
djaglowski,
dmitryax,
JonathanWamsley,
MovieStoreGuy,
Mrod1598 and
schmikei
|
@dmitryax This was intended to be a solution, but if someone else had another way of doing it thats cleaner or adds more functionality in the long run, then this could be a temporary. I just wanted to try to get this in so when I go through and and do #21213, I can also handle the changing of the typeStr on the receivers. |
| @@ -7,6 +7,6 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| Type = "activedirectorydsreceiver" | |||
| Type = "activedirectoryds" | |||
There was a problem hiding this comment.
The type registered in factory.go is active_directory_ds. Unfortunately, this change is not enough to accommodate.
There was a problem hiding this comment.
Ah, you are right. This one is a bit strange with the underscores. Do you have any solutions to this in mind, I can take a look into implementing a fix if you have an idea.
There was a problem hiding this comment.
No, for now, I lean towards taking your fix, and we will need to iterate one more time for this component and a few others.
There was a problem hiding this comment.
Another solution: align type in metadata.yaml with what we need, like active_directory_ds in this case. And use the directory name for Scope.Name. This solution will work even with #21469
There was a problem hiding this comment.
@dmitryax Just so I understand correctly, you are talking about going into active_directory_ds's metadata.yaml then changing the Type from active_directory_ds into something like activedirectoryds?
There was a problem hiding this comment.
No, we should keep the type as is. Otherwise, it'll be a significant breaking change. I mean, we set active_directory_ds in metadata.yaml to have it rendered here. But the #21382 can be solved by setting the directory in the Scope.Name field instead of this type
There was a problem hiding this comment.
Updated link in my latest comment ^
|
Please rebase off main to pick up the fix when checking licenses. |
a bit more efficient and clean?
BominRahmani
force-pushed
the
mdatagen-hack-metadata
branch
from
39a9c31 to
8dfcc7a
Compare
|
Why do we need to merge this PR if the generated type cannot be used yet? |
|
I think I can make it work for this PR: #21275 |
|
#21275 already have this part on the query/validation path https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/21275/files#diff-35261d8aa538a618698be27fbb71cd350cdf5db0f9693652ceaa151b26a78409R57 ? :) |
|
True, I can make it work either way, I could drop that code though if this one comes in. |
|
I submitted #21501. Once it's merged, you can set the actual component type in the metadata.yaml |
|
Superseded by #21382 |
Description: Changed what mdatagen outputs into the generated_status_go file for receivers. If the receiver has the suffix "receiver", it will get rid of that. This will allow us to also get rid of the "typeStr" variable for most of the receivers and standardize it into metadata.Type. The same way we have been doing for stability -> metadata.Stability
Within this change is also the make generate update for generated_status_go files for the receivers that have had that bit added in.