The change we did to logs field mapping for reference -

"logs": {
          "dynamic": false,
          "type": "object",
          "enabled": true,
          "properties": {
            "fields": {
              "dynamic": false,
              "type": "object",
              "enabled": true,
              "properties": {
                "type": {
                  "type": "text",
                  "fields": {
                    "keyword": {
                      "ignore_above": 256,
                      "type": "keyword"
                    }
                  }
                },
                "value": {
                  "type": "text",
                  "fields": {
                    "keyword": {
                      "ignore_above": 256,
                      "type": "keyword"
                    }
                  }
                },
                "key": {
                  "type": "text",
                  "fields": {
                    "keyword": {
                      "ignore_above": 256,
                      "type": "keyword"
                    }
                  }
                }
              }
            },
            "timestamp": {
              "type": "long"
            }
          }
        }

@pavolloffay @yurishkuro - Any thoughts on this issue?

This sounds reasonable to me as Jaeger doesn't provide functionality to search in log fields, just tags and I believe this is where the benefits of nested fields lay. As long as there are no changes in behaviour in Jaeger UI in the logs display, which you mentioned is the case from your comments in Slack.

Are you able to contribute a PR for this?

Sure I would raise one. If I am not wrong only places that require change are here (If no tests break due to this) -

and

as Jaeger doesn't provide functionality to search in log fields

It certainly does with Cassandra, and I thought with ES too

@albertteoh - My Integration tests are failing and I could see why. We are actually using nested Queries for logs as well.

I could see while building Query for fetching traceIDs - We are adding a BoolQuery for tags, as well as Logs.

Not sure why we are trying query for logs with the key value pairs of tags.

I thought tags and logs are independent fields. Also I am unsure how our Jaeger Query UI was able to fetch traceIDs properly after the mapping change - Ideally it should fail as the integration tests are ( with 400 Bad Request) as we are trying to run nested query on a non-nested field.

This would need further investigation. Kindly share your thoughts too

Thanks for the correction @yurishkuro @bhiravabhatla :)

Tested the app locally. As suspected, if add any tags while searching traces - we are getting 400 Bad request from ES.

@albertteoh @yurishkuro - I am trying to understand why we are using the key/values passed in Tags from UI for querying Logs field as well -

	for i := range objectTagFieldList {
		queries[i] = s.buildObjectQuery(objectTagFieldList[i], kd, v)
	}
	for i := range nestedTagFieldList {
		queries[i+objectTagListLen] = s.buildNestedQuery(nestedTagFieldList[i], k, v)
	}


	// but configuration can change over time
	return elastic.NewBoolQuery().Should(queries...)

Basically we adding Should Query for matches in each of tag objectfield, tagsNested field, Process.tag field as well as logs.fields. I can understand looking for matches in other tag fields - but little confused about log field. This would imply the trace would be returned if the key,value mentioned tags matches any of the logs field key/value pairs as well.

@pavolloffay @yurishkuro @albertteoh - Any thoughts on this?

This would imply the trace would be returned if the key,value mentioned tags matches any of the logs field key/value pairs as well.

@bhiravabhatla yes, that's correct. For example, if you try the HotRod demo with Jaeger all-in-one running, if your Tag search is event="Found drivers", it will result in traces containing at least one span with a log (or tag/process) entry key of event and value of Found drivers.

Is this expected?. Ideally we should be only looking only in tags. At least as an end user doing the search from Query UI, I would expect that the search should only happen in tag fields for filtering.

@bhiravabhatla This was before my time, so I'll defer to the more experienced members of the community on this question.

I agree though that Jaeger UI's search field for this is labelled "Tags", so I can see how this could be misleading, especially if this behaviour is not consistent for all backends (i.e. limited to Cassandra and ES).

As an aside, maybe log search capabilities for specific storage backends should be surfaced in Jaeger UI's help dialog?

If we were to remove log search support from Cassandra and ES, it would be a breaking change for folks who may be aware of, and actively rely on, this capability.

@albertteoh @pavolloffay - Should we not rather have consistent behaviour across all backends?

If we want logs to be searchable from UI - Atleast it should be behind a toggle - so that I can choose to have it. Problem is these fields are taking up too much memory - causing the ES nodes to do full GC regularly - resulting in span drops.

We are not using these fields for search at all - so we dont need them to be cached at all.

If we were to remove log search support from Cassandra and ES, it would be a breaking change for folks who may be aware of, and actively rely on, this capability.

Also I don't see this documented. I guess if we decide to keep it we would need to document it somewhere as well.

@bhiravabhatla I'd agree that ideally there should be consistent behaviour across all backends, and perhaps this inconsistency was due to limitations in later introduced backends that couldn't meet the requirements.

Atleast (sic) it should be behind a toggle

I think this is a reasonable request, esp. given the improvements in ES memory use. Not sure what other folks think @jaegertracing/elasticsearch?

I imagine we'd need to add a "disable-log-indexing" flag so that the collector knows to exclude nesting from its mapping file, and also to remove log searches from jaeger-query's ES query.

Also I don't see this documented. I guess if we decide to keep it we would need to document it somewhere as well.

Either that, or surface it in the UI so it's clearer to the user; but that would mean a bigger change to expose backend capabilities via the API.

Although, having the API expose capabilities may be useful for some use cases like the one mentioned in the jaeger channel relating to searches across services. In this case, if jaeger-query could convey that the backing store supports such searches, then perhaps jaeger UI would be able to allow queries without service/operations being set.

A good alternative to both object and nested types is the new flattened type which is mostly designed for this kind of use: representing tags and labels.
https://www.elastic.co/guide/en/elasticsearch/reference/current/flattened.html
It's the best of both worlds, as it prevents both dynamic field mapping explosion (like nested/unlike object) and expensive indexing/querying (like object/unlike nested).

@gquintana this looks interesting. I ll go through this & see how changing type to flattened impacts memory foot print. I ll try to get screenshots as I did for previous types.

Sorry could not spend time on this. Will try to spend sometime over this weekend.

A good alternative to both object and nested types is the new flattened type which is mostly designed for this kind of use: representing tags and labels. https://www.elastic.co/guide/en/elasticsearch/reference/current/flattened.html It's the best of both worlds, as it prevents both dynamic field mapping explosion (like nested/unlike object) and expensive indexing/querying (like object/unlike nested).

A drawback of flattened in regards to nested is however the following, taken directly from the docs of https://www.elastic.co/guide/en/elasticsearch/reference/current/nested.html

The user.first and user.last fields are flattened into multi-value fields, and the association between alice and white is lost. This document would incorrectly match a query for alice AND smith:

@bhiravabhatla I'd agree that ideally there should be consistent behaviour across all backends, and perhaps this inconsistency was due to limitations in later introduced backends that couldn't meet the requirements.

Atleast (sic) it should be behind a toggle

I think this is a reasonable request, esp. given the improvements in ES memory use. Not sure what other folks think @jaegertracing/elasticsearch?

I imagine we'd need to add a "disable-log-indexing" flag so that the collector knows to exclude nesting from its mapping file, and also to remove log searches from jaeger-query's ES query.

Also I don't see this documented. I guess if we decide to keep it we would need to document it somewhere as well.

Either that, or surface it in the UI so it's clearer to the user; but that would mean a bigger change to expose backend capabilities via the API.

The searching for Logs (and possible limitations) should definitely be described (better) somewhere. I still don't know why I can search some logs, and some others not: #3046

Restarting this thread :).

Finally got some time to work on this. Have added a flag to support disabling the search on logs (false by default). Once enabled the mapping for the logs field would be set to object (vs original nested type). Also made sure once this flag is enabled - spanstore reader does not search for key/values in logs field. Search on Tags and process field would continue as usual.
Went through the flattened data type too - It has its own limitations on queries - no tokenization and regex support. So instead, thought of adding a flag to disable search in logs all together. I believe there would be jaeger users out there who simply dont need search capability on logs field.

Have changed the mapping and reindexed one our prod jaeger span index. Please check the results below:
With old mapping [nested logs]:

With new mapping[ object type logs]

From ES monitoring -
Before

After

Have tested this in our lower environments. Tags search continues to work as expected and other functionalities are intact too -

@albertteoh @yurishkuro @pavolloffay - please take a look and share feedback.

@yurishkuro - Could you please take a look into this.