-
Notifications
You must be signed in to change notification settings - Fork 106
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
Initial support for EoIP, GRE and IPIP #206
Conversation
@akpw this PR adds the disabled label to interface_status. This might break backward compatibility. Should we add an additional metric like |
@phibos adding a label should be fine, however dropping existing labels or renaming existing metrics would result in breaking compatibility. from a quick glance there seems to be the case with |
Current status metrics
Status metrics after applying the changes:
From a quick diff, the following changes:
Current version: The reported status is the status value from New version: The status is the running value reported by From the docs: https://help.mikrotik.com/docs/spaces/ROS/pages/8323191/Ethernet#Ethernet-Properties
At the moment I can't see any difference in the status reported. But if you like to keep the |
One thing we should change is the handling of the For some metrics it is handled in the interface_ds ... mktxp/mktxp/datasource/interface_ds.py Lines 72 to 75 in 3beb00e
... and for some others it is handled by the interface_collector mktxp/mktxp/collector/interface_collector.py Lines 32 to 34 in 3beb00e
After this PR is merged I can proved a new PR to move the |
@phibos thanks for the thorough check. Looks like the docs indicate there could be a difference, and e.g. the default dashboard already uses the link status context as well. So yes if possible it'd be better to leave |
* Splitting the status and running metrics * interface_status is the link status * interface_running is the running state reported by RouterOS devices * interface_disabled is the disabled state * Move the handling of the 'use_comments_over_names' setting to the DS, now it is also supported for GRE, IPIP, EoIP and other interfaces
083aee0
to
5d821f7
Compare
@akpw I have
|
This adds initial support for EoIP, GRE and IPIP interfaces and fixes #204