Skip to content
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

Not able to publish any new metric via NDATA or NCMD which is not available in NBIRTH. #85

Open
malavvvakharia opened this issue Mar 12, 2024 · 12 comments
Assignees
Labels
duplicate This issue or pull request already exists

Comments

@malavvvakharia
Copy link

malavvvakharia commented Mar 12, 2024

Hello @SeppPenner ,
According to the sparkplug specification, we can also send the not known metric in NDATA and NCMD. However, from the code, it is not possible as it will filter the data and remove all unknown metrics before publishing NCMD or NDATA.

SS of code:

image

SS of specification:

image

image

@SeppPenner SeppPenner self-assigned this Mar 13, 2024
@SeppPenner SeppPenner added the duplicate This issue or pull request already exists label Mar 13, 2024
@SeppPenner
Copy link
Owner

Basically a duplicate of #44 and #53.

@malavvvakharia
Copy link
Author

Yes, it's partially connected with both tickets but not directly because in this ticket I mentioned that if you add any new metric that is not available in NBIRTH and try to publish that metric via NDATA or NCMD it will not pass it to HostApp as it's an unknown metric which you are filtering via the above-mentioned method.

@SeppPenner
Copy link
Owner

For me, the spec is misleading in this case. E.g. check DBIRTH:

6.4.20. DBIRTH
The DBIRTH is responsible for informing the Host Application of all of the information about the
device. This includes every metric it will publish data for in the future.

Same for NBIRTH. For me, this is not concludent whether all metrics need to be known in advance or could be added like this...

@SeppPenner
Copy link
Owner

SeppPenner commented Mar 14, 2024

I have asked the tahu team in eclipse-tahu/tahu#370, but I guess I have an idea how this should be read:

Imagine a scenario with this library and another one that speak Sparkplug. Let's say the other library allows to send unknown metrics on NDATA or DDATA --> Theoretically, this library would need to tell these nodes to do a rebirth because the metrics are unknown. Therefore, the filtering / excluding of unknown metrics in this library is correct.

@malavvvakharia
Copy link
Author

malavvvakharia commented Mar 15, 2024

@SeppPenner ,
According to this ticket: #58 and #61 I think you have removed the logic of requiring the known metrics and also validating the same.

For me, the spec is misleading in this case. E.g. check DBIRTH:

6.4.20. DBIRTH
The DBIRTH is responsible for informing the Host Application of all of the information about the
device. This includes every metric it will publish data for in the future.

Same for NBIRTH. For me, this is not concludent whether all metrics need to be known in advance or could be added like this...

@adityashahazilen
Copy link
Contributor

adityashahazilen commented Mar 18, 2024

Therefore, the filtering / excluding of unknown metrics in this library is correct.

Agree with your statement "Theoretically, this library would need to tell these nodes to do a rebirth because the metrics are unknown". However as per sparkplug specification library should send rebirth request to edge node from host application not filter unknown metric. I would not agree on statement "Therefore, the filtering / excluding of unknown metrics in this library is correct."

@malavvvakharia
Copy link
Author

@SeppPenner , have you checked it? Do you have any updates on it?

@SeppPenner
Copy link
Owner

I will keep this open until the TAHU team answers, but for the moment I will go with filtering.

@zhcdorn
Copy link

zhcdorn commented Mar 28, 2024

We are also thinking about Sparkplug and as we are .NET team interested in this library as it seems to have become really applicable over last weeks thanks to all your contributions. However, for us, in order that it is "scalable" we need this filtering deactivated. Knowing all metrics in advance creates a huge dependency on metrics, and as we have edge nodes with changing metrics (e.g. new versions of edge nodes) we need to "auto update" metrics anytime, anywhere.

Can we not agree on a flag to allow either case? (filtering on/off)

@zhcdorn
Copy link

zhcdorn commented Apr 5, 2024

I will keep this open until the TAHU team answers, but for the moment I will go with filtering.

Any news from TAHU team so far? Thx

@adityashahazilen
Copy link
Contributor

Any update from TAHU team ? I see we are at now version 1.8.0 but we cannot use it with this filter.

@SeppPenner
Copy link
Owner

This is valid. From eclipse-tahu/tahu#370:

Known means known. If you send an NDATA/DDATA with a new Metric, that would be invalid and the Host Application will require you to send a new NBIRTH/DBIRTH that contains your new Metric in order to move forward. In other words, sending an NDATA/DDATA with a new metric is wrong and should not be done.

So, to clearify this again: The behaviour is expected and if you want to send new / unknown metrics, you have to do a device death / device birth or node death / node birth aka "rebirth" with the new metrics list in it.

--> For the project, this means that I will not change the behaviour of the filtering, but will add a new method like "Rebirth" for nodes and devices to simplify the process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants