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

Have annotations work with domain objects that have dots #7063

Closed
1 of 7 tasks
scottbell opened this issue Sep 18, 2023 · 5 comments · Fixed by #7065
Closed
1 of 7 tasks

Have annotations work with domain objects that have dots #7063

scottbell opened this issue Sep 18, 2023 · 5 comments · Fixed by #7065
Labels
Milestone

Comments

@scottbell
Copy link
Contributor

scottbell commented Sep 18, 2023

Summary

Annotations that have periods in their domain object IDs (like YAMCS aggregated telemetry) aren't retrievable with CouchDB. Periods are a special character in JSON. Namely the targets of the annotations look like this:

"targets": {
      "taxonomy:~myproject~Gyro.x": {
        "minX": 1694597219934.105,
        "minY": 13.14969686405388,
        "maxX": 1694597312669.8083,
        "maxY": 29.724520818440027
      },
      "taxonomy:~myproject~Gyro.y": {
        "minX": 1694597219934.105,
        "minY": 13.14969686405388,
        "maxX": 1694597312669.8083,
        "maxY": 29.724520818440027
      },
      "taxonomy:~myproject~Gyro.z": {
        "minX": 1694597219934.105,
        "minY": 13.14969686405388,
        "maxX": 1694597312669.8083,
        "maxY": 29.724520818440027
      }
    }

The taxonomy:~myproject~Gyro.y is causing the CouchDB query fits because it's interpreting y (and x and z) as sub-properties of the target instead as part of the domain ID.

Unfortunately Couch doesn't allow regex on the field name during search, so we can't just escape the dot in the query. The solution we're going to pursue is refactoring the annotation to look like this:

"targets": [
    {
        "keyString": "taxonomy:~myproject~Gyro.x"
        "minX": 1694597219934.105,
        "minY": 13.14969686405388,
        "maxX": 1694597312669.8083,
        "maxY": 29.724520818440027
      }
    ]

Namely, make the target an array (instead of an object).

Expected vs Current Behavior

Annotations should be able to work on aggregated telemetry in YAMCS.

Steps to Reproduce

  1. Find an aggregate piece of telemetry (like Gyro in YAMCS Quickstart).
  2. Drop a piece of the aggregated telemetry onto a Overlay Plot (e.g., Gyro.x)
  3. Attempt to tag some data
  4. Note that the tag never takes, though the tagged area is searchable by GrandSearch, the points never show up

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?
  • Does this block the execution of e2e tests?
  • Does this have an impact on Performance?

Additional Information

@scottbell scottbell self-assigned this Sep 18, 2023
@scottbell
Copy link
Contributor Author

I think with the restructured annotations, we've got a solution:

Screen.Recording.2023-09-18.at.4.51.39.PM.mov

@scottbell
Copy link
Contributor Author

scottbell commented Sep 18, 2023

@akhenry I created 16000 annotations, and 32000 objects in a CouchDB database. Without an index, querying for tags (e.g., using GrandSearch to look for "Science":

{
   "selector": {
      "$and": [
         {
            "model.type": {
               "$eq": "annotation"
            }
         },
         {
            "model.tags": {
               "$elemMatch": {
                  "$in": [
                     "46a62ad1-bb86-4f88-9a17-2a029e12669d"
                  ]
               }
            }
         }
      ]
   }
}

executes in ~240ms.

Adding an index:

{
  "index": {
    "fields": ["model.type", "model.tags"]
  },
  "name": "type_tags_index",
  "type": "json"
}

has the query execute in 21ms, an 11x speedup. On a miss though (e.g., no hit on a tag), we're still looking at a very slow retrieval - ~2s.

We can improve speed more by using design documents. These reduce lookup time on tags by ~10x, even on misses. For general annotation searches for a target, the query goes from 4s to 5ms - 800x speedup. The downside is they increase the raw size of the database, take more time to make/update. Though in our case, this is probably acceptable.

We can also add a design document for retrieving domain objects for specific tags:

{
  "_id": "_design/annotation_tags_index",
  "views": {
    "by_tags": {
      "map": "function (doc) { if (doc.model && doc.model.type === 'annotation' && doc.model.tags) { doc.model.tags.forEach(function (tag) { emit(tag, doc._id); }); } }"
    }
  }
}

and can be retrieved by issuing a GET to http://localhost:5984/openmct/_design/annotation_tags_index/_view/by_tags?key="TAG_ID_TO_SEARCH_FOR"&include_docs=true
where TAG_ID_TO_SEARCH_FOR is the tag UUID we're looking for.

and for targets:

{
  "_id": "_design/annotation_keystring_index",
  "views": {
    "by_keystring": {
      "map": "function (doc) { if (doc.model && doc.model.type === 'annotation' && doc.model.targets) { doc.model.targets.forEach(function(target) { if(target.keyString) { emit(target.keyString, doc._id); } }); } }"
    }
  }
}

and can be retrieved by issuing a GET to http://localhost:5984/openmct/_design/annotation_keystring_index/_view/by_keystring?key="KEY_STRING_TO_SEARCH_FOR"&include_docs=true
where KEY_STRING_TO_SEARCH_FOR is the UUID we're looking for.

To accommodate these, I'm going to add an option to the CouchDB plugin to use these design documents if available.

@khalidadil
Copy link
Contributor

Verified Fixed in Testathon on 09/22/23. I was able to add annotations to telemetry with a dot in the name as well as search and pivot back to the annotated points.

@rukmini-bose
Copy link
Contributor

Verified Testathon 9/23/2023, looks good!

@ozyx
Copy link
Contributor

ozyx commented Sep 22, 2023

  • Verified the annotation removal script works on testathon

  • Verified that plot annotations work with aggregate members

  • Verified that map annotations work with the new format

  • Verified that image annotations work with the new format (bug found)

  • Verified that notebook annotations work with the new format

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants