-
Notifications
You must be signed in to change notification settings - Fork 104
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
Create SlicingDice sink #1542
base: master
Are you sure you want to change the base?
Create SlicingDice sink #1542
Conversation
@@ -9,3 +9,4 @@ | |||
[cygnus-ngsi][KafkaSink] Using global connection to zookeeper instead of creating one each time an event arrives | |||
[cygnus-ngsi][NGSINameMappingsInterceptor] Now namemapping checks sevice, subervice and (type of entity and id entity) of EntityMapping (#1535) | |||
[cygnus-ngsi][NGSIEvent] Unable to deliver event: null pointer getAttributeForNaming (#1506) | |||
[cygnus-ngsi][SlicingDiceSink] Added SlicingDice sink to cygnus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue number should be cited at the end of the line (check other lines in the same file, please).
Small typo: cygnus -> Cygnus ;)
public class SlicingDiceBackendImplTest { | ||
|
||
// constants | ||
private static final String DATABASE_KEY = "oiasdiondasidasndasomn"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fake key? (just to check)
* @param in | ||
* @return The encoded string | ||
*/ | ||
public static String encodeSlicingDice(String in) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
97, 119, 121, 122, 48, 57...
They may appera as magic number for people not very familiar with encoding. Maybe it would be a good idea to include a little explanation on how this function works in the JavaDoc preamble (explaining the difreent ranges/exceptions).
|
||
public static class PersistBatchTest { | ||
|
||
private static final String DATABASE_KEY = "oiasdiondasidasndasomn"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fake key? (just checking)
NGSISlicingDiceSink sink = new NGSISlicingDiceSink(); | ||
sink.setPersistenceBackend(mockBackend); | ||
|
||
final String databaseKey = "oasdisadnasoi"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fake key? (just checking)
@@ -139,6 +141,20 @@ ENV CYGNUS_POSTGRESQL_BATCH_TIMEOUT "" | |||
ENV CYGNUS_POSTGRESQL_BATCH_TTL "" | |||
ENV CYGNUS_POSTGRESQL_ENABLE_CACHE "" | |||
|
|||
# SlicingDice options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New env vars should be documented in the proper documentation file (more on this in a general comment I'll provide soon).
@@ -180,6 +182,19 @@ cygnus-ngsi.sinks.orion-sink.keystone_ssl = false | |||
cygnus-ngsi.sinks.orion-sink.orion_fiware = | |||
cygnus-ngsi.sinks.orion-sink.orion_fiware_path = | |||
|
|||
|
|||
# SlicingDiceSink configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New configuration parameter (i.e. cygnus-ngsi.sinks.slicingdice*) should be documented in the proper documentation file (more on this in a general comment I'll provide soon).
Thank you very much for the contribution! I have had a look to the PR. With regards to code I haven't performed an in deep review (I'm afraid I lack the knowledge in SliceDice to do that ;). I have only look for kind of "formal" aspect, with some minor comments regarding it. As long as the new sink doesn't break the existing e2e regression (we would check upon merging), that's ok. My main piece of feedback is regarding documentation. The work you have done needs to be properly and comprehensively documented in the PR along with the code and unit test contribution. This includes:
This list could not be exahustive. It would be great if you could have a look to the Cygnus .md files in general in the repository to find any other possible directory/file that could need additions and I have missed, please. Thanks! |
Hi @fgalan, Thanks so much for the review and all the comments. We will correct everything ASAP and let you know. Regards, |
@joaosimbiose any update in this PR, please? Don't hesitate of asking if something is not clear about what is pending in this PR to be finished and merged. Thanks! |
This PR resolves the issue #1541
The SlicingDice's development team wrote this Cygnus extension for a new sink to persist data in SlicingDice's databases.
Please, we kindly ask you to review this pull request and provide us your insights and considerations about the implementation.
We read and followed all the contribution guidelines [1], documenting the code and also creating all the necessa1y tests.
[1] - https://github.com/telefonicaid/fiware-cygnus/blob/master/doc/contributing/contributing_guidelines.md#contributing-guidelines
Thanks in advance.
SlicingDice API Overview
SlicingDice provides a REST API [2] that allows its customers to insert (and query) data using the API.
In order to insert values into a SlicingDice database, you send a POST request to
https://api.slicingdice.com/v1/insert
with a JSON object which the key is theentity ID
containing JSON objects with database's column names and their respective values.In the Cygnus case, our implementation considers the
entity ID
to be the IoT device generating the data.Any timeseries-related data (metrics) sent by Cygnus will be stored in a SlicingDice database on an
Event Column
and the non-timeseries data on anAttribute Column
.An example of the API insertion request generated by this Cygnus extension would look like this:
Implementation considerations:
"auto-create": ["dimension", "column"]
at the end of all insertion requests. This allows this integration to never break in case Cygnus adds new columns.[2] - SlicingDice's API Docs are available at https://docs.slicingdice.com
Implementation Overview
feature
[cygnus-common] com.telefonica.iot.cygnus.backends.slicingdice.SlicingDiceBackendImpl
auto_create=true
configuration.[cygnus-ngsi] com.telefonica.iot.cygnus.sinks.NGSISlicingDiceSink
Dockerfile, agent.conf and cygnus-entrypoint.sh