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

store navigationTiming marks as scaled floats #915

Merged
merged 2 commits into from
Jun 15, 2018

Conversation

graphaelli
Copy link
Member

@graphaelli graphaelli commented May 7, 2018

for #708. Amends the elasticsearch mapping template to include:

{
  "transaction.marks.navigationTiming": {
    "mapping": {
      "type": "scaled_float",
      "scaling_factor": 1000000
    },
    "match_mapping_type": "*",
    "path_match": "transaction.marks.navigationTiming.*"
  }
}

Given this event:

{
  "@timestamp": "2017-05-30T18:53:27.100Z",
  "transaction": {
    "marks": {
      "navigationTiming": {
        "appBeforeBootstrap": 608.9300000000001,
        "navigationStart": -21
      }
    }
  }
}

the current elasticsearch mapping would be:

{
  "marks": {
    "properties": {
      "navigationTiming": {
        "properties": {
          "appBeforeBootstrap": {
            "type": "float"
          },
          "navigationStart": {
            "type": "long"
          }
        }
      }
    }
  }
}

with this change, the mapping ends up:

{
  "marks": {
    "dynamic": "true",
    "properties": {
      "navigationTiming": {
        "dynamic": "true",
        "properties": {
          "appBeforeBootstrap": {
            "type": "scaled_float",
            "scaling_factor": 1000000
          },
          "navigationStart": {
            "type": "scaled_float",
            "scaling_factor": 1000000
          }
        }
      }
    }
  }
}

This is a special case update to store navigationTimings.

@graphaelli graphaelli force-pushed the navigation-timing-template branch 2 times, most recently from 2efea2b to 623ade8 Compare May 7, 2018 20:05
@graphaelli graphaelli added this to the 6.4 milestone May 7, 2018
@graphaelli graphaelli self-assigned this May 7, 2018
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for a short term solution.
As transaction.marks can hold all kinds of objects we should investigate how we can enable this for whatever values are sent in the future.

@jalvz
Copy link
Contributor

jalvz commented Jun 7, 2018

are we waiting for something to merge this?

@graphaelli
Copy link
Member Author

I should have written it up here, sorry. Per @simitt's comment, I could not confirm that this change doesn't affect other objects under transaction.marks. I think an integration test would be a good addition before merging.

@graphaelli graphaelli force-pushed the navigation-timing-template branch from 623ade8 to 571ac0e Compare June 14, 2018 20:54
@graphaelli
Copy link
Member Author

integration test data - @jalvz @simitt mind checking it out?

@@ -119,6 +119,10 @@
},
"id": "945254c5-67a5-417e-8a4e-aa29efcbfb79",
"marks": {
"another_mark": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to run approvals again, these are now called some_long some_float

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, done

@graphaelli graphaelli force-pushed the navigation-timing-template branch 2 times, most recently from c5c2bf8 to 03834a5 Compare June 15, 2018 13:03
@graphaelli graphaelli force-pushed the navigation-timing-template branch from 03834a5 to 860e3ea Compare June 15, 2018 15:00
@graphaelli graphaelli merged commit acbdc3c into elastic:master Jun 15, 2018
@graphaelli graphaelli deleted the navigation-timing-template branch June 15, 2018 15:13
simitt pushed a commit to simitt/apm-server that referenced this pull request Jul 4, 2018
* store navigationTiming marks as scaled floats

* test that navigationTiming marks are the only ones impacted
@simitt simitt mentioned this pull request Jul 4, 2018
simitt pushed a commit to simitt/apm-server that referenced this pull request Jul 4, 2018
* store navigationTiming marks as scaled floats

* test that navigationTiming marks are the only ones impacted
simitt pushed a commit that referenced this pull request Jul 4, 2018
* store navigationTiming marks as scaled floats

* test that navigationTiming marks are the only ones impacted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants